Skip to content

fix 10k file test on appveyor#5528

Merged
shiftkey merged 2 commits intomasterfrom
appveyor-fails-silently
Sep 5, 2018
Merged

fix 10k file test on appveyor#5528
shiftkey merged 2 commits intomasterfrom
appveyor-fails-silently

Conversation

@outofambit
Copy link
Copy Markdown
Contributor

@outofambit outofambit commented Aug 31, 2018 •
edited
Loading

fixes #5527

The solution for this ended up being --detectOpenHandles, which gave me reliable failures for the 10k untracked file test. This test was only failing because it timed out, so I increased the time limit for that one test.

thanks @shiftkey and @streeter!

@outofambit outofambit mentioned this pull request Aug 31, 2018
Closed
@shiftkey
Copy link
Copy Markdown
Member

shiftkey commented Sep 4, 2018

This seems to be the expected behaviour from that last build:

  • jest test suite reports an error, returns exit code 1 (because one failure)
  • yarn detects this non-zero exit code and returns that
  • Appveyor build runner detects yarn non-zero exit code and

screen shot 2018-09-04 at 10 00 58 am

The build linked to in #5527 shows a different sort of error - timeout/EMFILE errors - and we don't see jest return in the same way - it just continues on to the next test suite:

Perhaps we need to set this flag?

screen shot 2018-09-04 at 10 04 35 am

@outofambit outofambit changed the title make tests fail make tests (not) fail Sep 4, 2018
@outofambit outofambit force-pushed the appveyor-fails-silently branch from 09f1d5a to 1a32733 Compare September 5, 2018 16:53
@outofambit outofambit changed the title make tests (not) fail fix 10k file test on appveyor Sep 5, 2018
@outofambit outofambit added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 5, 2018
outofambit
outofambit commented Sep 5, 2018
View reviewed changes
app/test/unit/git/status-test.ts
expect(files.length).to.equal(numFiles)
},
// needs a little extra time on CI
25000

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@shiftkey shiftkey self-assigned this Sep 5, 2018
@shiftkey shiftkey merged commit 218122b into master Sep 5, 2018
@j-f1 j-f1 mentioned this pull request Sep 5, 2018
Closed
@shiftkey shiftkey deleted the appveyor-fails-silently branch September 5, 2018 20:42
SimenB
SimenB reviewed Sep 7, 2018
View reviewed changes
package.json
"cli": "ts-node --require ./app/test/globals.ts --require ./app/src/cli/dev-commands-global.ts app/src/cli/main.ts",
"test:integration": "cross-env TEST_ENV=1 ELECTRON_NO_ATTACH_CONSOLE=1 xvfb-maybe --auto-servernum -- mocha -t 30000 --require ts-node/register app/test/integration/*.ts",
"test:unit": "cross-env ELECTRON_RUN_AS_NODE=1 node_modules/.bin/electron ./node_modules/jest/bin/jest --silent --config ./jest.config.js",
"test:unit": "cross-env ELECTRON_RUN_AS_NODE=1 node_modules/.bin/electron ./node_modules/jest/bin/jest --detectOpenHandles --silent --config ./jest.config.js",

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@outofambit outofambit mentioned this pull request Sep 27, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@SimenB SimenB SimenB left review comments

@j-f1 j-f1 j-f1 left review comments

@shiftkey shiftkey shiftkey approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@shiftkey shiftkey

Labels

ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@outofambit @shiftkey @SimenB @j-f1