Skip to content

Conversation

@deivid-rodriguez
Copy link
Contributor

What was the end-user or developer problem that led to this PR?

Unfortunately I couldn't find a non-realworld scenario when introducing these specs to prove fixes, and they were specific to ruby 2.7 versions.

After dropping Ruby 2.7 support, coverage was lost.

What is your fix for the problem, implemented in this PR?

For one of them, I'm assuming the coverage loss because the Gemfile was very particular to Ruby 2.7, and I wouldn't know how to do better now, the fix happened a long time ago. We could try patching the current ruby to fake it's Ruby 2.7, but not sure if it's worth the effort.

For the other one, I moved it to a spec that runs for all rubies and just makes sure that the Gemfile can be resolved in "reasonable" time.

Make sure the following tasks are checked

@deivid-rodriguez deivid-rodriguez force-pushed the handle-old-realworld-specs branch 2 times, most recently from 2e7ee96 to cd0e35f Compare December 15, 2023 19:55
Unfortunately I couldn't find a non-realworld scenario when introducing
these specs to prove fixes, and they were specific to ruby 2.7
versions.

After dropping Ruby 2.7 support, coverage was lost.

For one of them, I'm assuming the coverage loss because the Gemfile was
very particular to Ruby 2.7, and I wouldn't know how to do better now,
the fix happened a long time ago. We could try patching the current ruby
to fake it's Ruby 2.7, but not sure if it's worth the effort.

For the other one, I moved it to a spec that runs for all rubies and just
makes sure that the Gemfile can be resolved in "reasonable" time.
@deivid-rodriguez deivid-rodriguez force-pushed the handle-old-realworld-specs branch from cd0e35f to 2440910 Compare December 15, 2023 21:25
@deivid-rodriguez
Copy link
Contributor Author

Also included a small change to improve output of specs that assert for "reasonable resolution time".

Happy to get suggestions on what's best here.

@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review December 15, 2023 21:26
Copy link
Contributor

@martinemde martinemde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

I read the actual code changes. They seem good.

@deivid-rodriguez
Copy link
Contributor Author

Thanks! Yeah, it's a big diff, but technically it's files that should've not been removed with #7116 if we wanted to keep the coverage. I guess it's good for now!

@deivid-rodriguez deivid-rodriguez merged commit 481a9ff into master Dec 19, 2023
@deivid-rodriguez deivid-rodriguez deleted the handle-old-realworld-specs branch December 19, 2023 12:32
deivid-rodriguez added a commit that referenced this pull request Dec 21, 2023
Handle realworld specs lost when dropping Ruby 2.7

(cherry picked from commit 481a9ff)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants