rspec-expectations icon indicating copy to clipboard operation
rspec-expectations copied to clipboard

100% test coverage

Open nevinera opened this issue 1 year ago • 4 comments

Well, aside from a few lines in two other PRs (https://github.com/rspec/rspec-expectations/pull/1457 and https://github.com/rspec/rspec-expectations/pull/1456) that needed non-spec changes.

First, I found and marked all of the segments that seem to be ruby-version specific, and set them off with :nocov: tags. That's in the first commit. Then I started tackling the remaining uncovered lines one at a time, adding tests I believe there to be no non-spec, non-comment changes in this PR, but I'm confident that some of the specs can be done more elegantly. Please, enhance my approaches, rspec wizards :-D

nevinera avatar May 08 '24 20:05 nevinera

The oldest rubies are failing two of the new tests:

  1) operator matchers RSpec::Matchers::BuiltIn::NegativeOperatorMatcher complains when negated
     Failure/Error: expect {
       expected /does not support `should_not != .*Use `should ==/ but nothing was raised
     # ./spec/rspec/matchers/built_in/operators_spec.rb:264

  2) operator matchers RSpec::Matchers::BuiltIn::PositiveOperatorMatcher complains when negated
     Failure/Error: expect {
       expected /does not support `should != .*Use `should_not ==/ but nothing was raised
     # ./spec/rspec/matchers/built_in/operators_spec.rb:241

I don't see anything obviously version dependent here, the only thing that seems plausible is this has_non_generic_implementation_of? method, but I don't see how variation there would cause these expectations to skip failing entirely..

I think this might mean those matchers are allowed and broken in 1.8.7? Which would probably be .. well, I doubt anybody on that version expects first-class support, but once I get 1.8.7 working in a container I can probably fix it at least :-)

In the meantime, I guess I should version-limit those tests? If I'm misunderstanding what's going on (which is likely, as I don't feel like I do understand what's going on yet), tell me more legends of how ruby used to be and I will learn again ;-)

nevinera avatar May 09 '24 17:05 nevinera

@pirj - The other two PRs are merged now, so I rebased this on top of main. That means that it actually has 100% coverage now - would you like me to bump this value up to 100% to keep it there, or leave it at 92% like it is now? I wasn't originally planning on doing so, but I noticed that we were actually trying to enforce a 100% value in rspec-core using such a config, so I thought I'd check.

nevinera avatar May 12 '24 02:05 nevinera

Let’s tighten it up to 100%, we can always loosen up if there’s something that we can’t reasonably cover.

pirj avatar May 12 '24 05:05 pirj

@JonRowe, this one has been waiting for a while, and ought to be fairly safe as it's test-only. I've updated it with latest if you have a minute to comment; apparently I was the only one to introduce uncovered code since the last push 6 weeks ago >.<

nevinera avatar Jun 30 '24 02:06 nevinera

Thank you!

pirj avatar Aug 18 '24 11:08 pirj