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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide more context when test fails due to timeout in `waitsFor` #17351

Merged
merged 1 commit into from May 18, 2018

Conversation

Projects
None yet
1 participant
@jasonrudolph
Member

jasonrudolph commented May 17, 2018

Prior to this change, if a test timed out using waitsFor, you ended up with a fairly vague error message:

timeout: timed out after 5000 msec waiting for something to happen

If a test has multiple waitsFor calls, that error message leaves you wondering which one failed. 馃槶

We ran into this problem when investigating #17325, in which the failing test included multiple places where the timeout could have occurred:

beforeEach(() => {
  socketServer = net.createServer(() => {})
  socketPath = path.join(rootDir1, 'some.sock')
  waitsFor(done => socketServer.listen(socketPath, done))   // DID IT TIME OUT HERE?
})

afterEach(() => waitsFor(done => socketServer.close(done))) // OR HERE?  馃し馃し馃し馃し馃し

One option is to pass a custom error message to each waitsFor call. While that's still possible, we'd like the default error message to give you a better chance of diagnosing the problem. With the changes in this pull request, if you don't pass a custom error message, waitsFor will show you the filename and line number where the timeout occurred.

Example

Before

socket files on #darwin or #linux
it ignores them
timeout: timed out after 5000 msec waiting for something to happen

After

socket files on #darwin or #linux
it ignores them
timeout: timed out after 5000 msec waiting for condition at afterEach (~/github/fuzzy-finder/spec/fuzzy-finder-spec.js:263:27)

Teach waitsFor to display filename + line number on timeout
If no error message is given, show the filename and line number when a
test fails due to a timeout using waitsFor.

Co-authored-by: Max Brunsfeld <maxbrunsfeld@github.com>

@jasonrudolph jasonrudolph merged commit 83e7441 into master May 18, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jasonrudolph jasonrudolph deleted the improve-message-when-test-fails-due-to-timeout branch May 18, 2018

jasonrudolph added a commit that referenced this pull request May 18, 2018

Merge pull request #17351 from atom/improve-message-when-test-fails-d鈥
鈥e-to-timeout

Provide more context when test fails due to timeout in `waitsFor`

@bittin bittin referenced this pull request Jul 25, 2018

Closed

1.28 releases #17733

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment