10/05/14 17:22, intrigeri wrote:
> I'm not 100% convinced by the way @skip works. I understand it can
> greatly speed up running the test suite, which is especially useful
> when working on it. But IMO, that's not the general case, and I'm
> quite sure it should not be treated this way by default (e.g.
> at release time). In the test frameworks I'm used to, here's how the
> TODO (known broken) tests work: a test case flagged as such is run,
> and then: if it fails, then it's result is ignored, and does not
> affect the overall result of the test suite; if it passes, then that's
> considered (maybe surprisingly) as a failure, which is actually
> logical since it most likely indicates that the test suite is
> outdated, or buggy. Without some similar logic, I'm concerned that we
> never think of unflagging tests marked as @skip, when we fix the
> corresponding bugs.
Ok, I get the problem you're describing and agree that the current
behaviour of @skip is bad.
For the record, I didn't implement the @skip mechanism for speed. To me
it just doesn't seem right to have tests that are expected to fail, and
let them fail all the time. In particular it would make pointless any
sort of automated run of these tests on a future server of ours that
email us whenever we commit something that breaks a test. It's true that
this probably is some distance into the future.
The reason I implemented it in this branch was that I wanted an easy way
to disable winxp.feature, but clearly the @skip mechanism isn't the
right solution.
> I realize that the current implementation (--tags
> ~@skip) is trivial, and that implementing the logic I'm suggesting
> above would likely be more involved (right?).
Indeed. I did a quick investigation.
cucumber provides the --wip switch, which almost does what you describe.
It will run everything normally, but in the end it will report all
passes as "unexpected", and exit with status 1 if there's any (so all
failures means exit status 0). It's not what we want, though, since we
cannot say that this behaviour only should affect scenarios with a
certain tag. Two separate runs (`--tags ~@todo` and `--wip --tags
@todo`) would be needed, but that would give two separate summaries,
which isn't good.
I've had a look at how an `Around` hook could be used for reversing the
pass/fail status. It would looks something like this:
Around('@todo') do |scenario, block|
# This runs the scenario
block.yield
# Now we try to reverse the pass/fail status
if scenario.failed?
print "Scenario failed as expected, treating it as a pass"
# how to change the status to :passed?
else
raise "This scenario should have failed."
end
end
However, I think we're at a dead since there is no way to "change the
status to :passed", as required, at least AFAICT. And I guess it'd be
error prone even if there was one. Also, note that any exceptions thrown
in our step definitions are caught and dealt with within `block`, so we
cannot catch and discard them at this level to fool cucumber that
nothing went wrong, or similar.
> So, how about 1.
> renaming @skip to @todo, and 2. supporting a --skip-todo *opt-in*
> switch, that does what you have implemented, and that we would *not*
> use without very good reasons, and certainly not at release testing
> time. What do you think?
I think we can just drop the whole @skip/@todo thing completely until we
have a mechanism that works like you describe above. This probably means
getting a new feature into upstream cucumber.
> Also, does "--tags ~@skip" conflict with any other usage of --tags
> that developers may find useful to (somehow) pass on the cucumber
> command-line themselves?
I don't think it would prevent anything meaningful (except perhaps
*only* running tests tagged @skip to see if they've been fixed). In
cucumber `--tags ~@skip --tags @foo` would result in the logical
conjunction of the two, so we'd run only scenarios tagged @foo but not
@skip.
> Maybe CUCUMBER_OPTS should not loseIn
> a potential existing value of itself, when initialized?
Sure. However, since I guess the code that introduced the variable will
be dropped, is that something you'd wished to be added any way?
Cheers!
