Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rspec-queue does not fail if there are NameErrors in specs #100

Open
eliotsykes opened this issue Jun 14, 2019 · 2 comments
Open

rspec-queue does not fail if there are NameErrors in specs #100

eliotsykes opened this issue Jun 14, 2019 · 2 comments

Comments

@eliotsykes
Copy link

eliotsykes commented Jun 14, 2019

I've just encountered the following behaviour with the rspec runner of ci-queue:

  1. Modify a spec to reference an uninitialized constant so as to trigger a NameError, e.g. by referring to a non-existent class:
describe MyNonExistentClass do
 ...
end
  1. Run rspec-queue
  2. rspec-queue exits successfully.

I guessed wrongly that rspec-queue would fail with a non-zero exit code, but it exited with 0.

Bare rspec exits with 1 in the above situation.

I'm wondering if this is intended behaviour - perhaps its needed for retries to work?

The error message output is:

NameError: uninitialized constant MyNonExistentClass

Debugging shows that line 400 is where the 0 exit code is returned, because the NameError causes @world.non_example_failure to be true. Syntax errors appear to have the same result:

success = true
@configuration.reporter.report(examples_count) do |reporter|
@configuration.add_formatter(BuildStatusRecorder)
@configuration.with_suite_hooks do
break if @world.wants_to_quit
queue.poll do |example|
success &= example.run(QueueReporter.new(reporter, queue, example))
break if @world.wants_to_quit
end
end
end
return 0 if @world.non_example_failure
success ? 0 : @configuration.failure_exit_code

@eliotsykes eliotsykes changed the title rspec-queue does not fail if there are syntax errors in specs rspec-queue does not fail if there are NameErrors in specs Jun 14, 2019
@casperisfine
Copy link
Contributor

I'm wondering if this is intended behaviour - perhaps its needed for retries to work?

So the answer is a bit of a yes and no.

yes: We do want to exit 0 if the worker crash on boot because of a transient issue (e.g. it fails to connect to the queue, or the machine it runs on is somehow unhealthy). Hence the behavior you noticed.

As for the no: Ideally this specific case should lead to an exit 1. If we can somehow discriminate it with certainty that it isn't a transient issue, then we could consider not soft failing it. That being said it's very hard to be sure the exception is caused by the loaded code, it could perfectly be that the spec code tries to connect to something when it's loaded (bad design but still).

All that being said, in theory it shouldn't be too much of a problem as long as you run the --report before succeeding your CI run. This not only centralize the error reporting. It also ensure that all the tests were ran (unless I'm wrong it should fail if 0 tests were found, if it doesn't then it's indeed a bug).

@eliotsykes
Copy link
Author

All that being said, in theory it shouldn't be too much of a problem as long as you run the --report before succeeding your CI run. This not only centralize the error reporting. It also ensure that all the tests were ran (unless I'm wrong it should fail if 0 tests were found, if it doesn't then it's indeed a bug).

Thanks for taking the time to explain @casperisfine, much appreciated! Until reading this I hadn't been running --report.

Though I see now its not needed with --report running, to fail faster on syntax errors, name errors, etc., I'm trying out running bin/rspec --dry-run first, which fails the CI build early. In case its of use to others here's what the script looks like (used on Heroku CI). (NB. For simplicity it runs the --report on every CI node instead of one, which seems to be OK so far.)

#!/usr/bin/env sh

# Perform a dry run first so all spec files are loaded but are not executed. rspec will exit with
# failure on any syntax etc. errors in the spec files.
bin/rspec --dry-run --format progress &&
  bundle exec rspec-queue --queue $REDIS_URL --timeout 300 &&
  bundle exec rspec-queue --queue $REDIS_URL --timeout 300 --report # Wait for test queue to be exhausted then print report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants