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

Test results and caching #352

Closed
wants to merge 13 commits into from
Closed

Test results and caching #352

wants to merge 13 commits into from

Conversation

zzak
Copy link
Contributor

@zzak zzak commented Mar 1, 2018

I've re-enabled caching and setup test summary support in the build UI.

Some builds for node version 8:

However, note the time is actually unaffected by cache, and each npm install step takes about ~9-10 seconds either way. I wonder why?

In any case, this is how you'd set up caching so feel free to adjust, or remove it (there's a penalty for uncached builds as you can see 8 seconds added for uploading them in this build.

Also, does the test summary look accurate?

Your build ran 317 tests in unknown with 0 failures

I'm not familiar with your suite so it may be missing some examples. Let me know.

Thanks!

@searls
Copy link
Member

searls commented Mar 2, 2018

Is there a way to matrix out node runtimes in a way that isn't so duplicative? Feel like first time I need to change something I'm going to fail to update all 3 runtimes

@zzak
Copy link
Contributor Author

zzak commented Mar 5, 2018

@searls Since steps is a map, you can either use a YAML anchor to reference an entire map that can be shared between jobs (like I was doing in the first PR). But you can't easily merge maps -- for example if you wanted a map of shared default steps, but some jobs have other steps.

The main problem we ran into here is the cache key for save_cache and restore_cache must be unique for each node version -- so it's coded into the step. Otherwise all of the other steps are the same.

I realize this is not great, there might be another way to achieve the caching without coding the node version in the key name, but not that I'm currently aware of.

At any rate, feel free to ping me however if you run into trouble with your build!

@zzak
Copy link
Contributor Author

zzak commented Mar 21, 2018

@searls Just following up on this, because I wasn't able to get a node-junit that worked with node 4 the test result config bits also had to be split out.

If node 4 wasn't included we could simplify things, here are the steps including test results (w/o caching):

    steps:
      - checkout
      - run: npm install tap-junit
      - run: npm install
      - run: npm run test:ci | ./node_modules/tap-junit/bin/tap-junit --output test_results --name junit
      - run: npm run cover:ci
      - store_artifacts:
          path: test_results
          prefix: test_results
      - store_test_results:
          path: test_results
      - store_artifacts:
          path: coverage
          prefix: coverage

@searls
Copy link
Member

searls commented Mar 22, 2018

Node 4 is EOL'd end of next month, so maybe that's the time to revisit this (we'll be dropping official 4.0 support on 4/30/18)

@searls
Copy link
Member

searls commented Apr 27, 2018

I dropped Node 4 today, does that mean this can be simplified and merged now?

@zzak
Copy link
Contributor Author

zzak commented Apr 27, 2018

@searls w00t! I will update the branch and rebase. 🙇

Zachary Scott added 3 commits April 27, 2018 13:19
These anchors are referenced in each node job, for steps and environment.
Since we can't share npm cache between node versions
@zzak
Copy link
Contributor Author

zzak commented Apr 27, 2018

@searls I have no idea what this error means, but can reproduce when I pipe npm run test:ci to tap-junit:

0 info it worked if it ends with ok
1 verbose cli [ '/usr/bin/node',
1 verbose cli   '/usr/lib/node_modules/npm/bin/npm-cli.js',
1 verbose cli   'run',
1 verbose cli   '--testdouble:build_file=dist/testdouble.js',
1 verbose cli   'test:example:plain-html' ]
2 info using npm@5.6.0
3 info using node@v8.11.1
4 verbose run-script [ 'pretest:example:plain-html',
4 verbose run-script   'test:example:plain-html',
4 verbose run-script   'posttest:example:plain-html' ]
5 info lifecycle testdouble@3.7.0~pretest:example:plain-html: testdouble@3.7.0
6 info lifecycle testdouble@3.7.0~test:example:plain-html: testdouble@3.7.0
7 verbose lifecycle testdouble@3.7.0~test:example:plain-html: unsafe-perm in lifecycle true
8 verbose lifecycle testdouble@3.7.0~test:example:plain-html: PATH: /usr/lib/node_modules/npm/node_modules/npm-lifecycle/node-gyp-bin:/home/zzak/code/testdouble.js/node_modules/.bin:/usr/lib/node_modules/npm/node_modules/npm-lifecycle/node-gyp-bin:/home/zzak/code/testdouble.js/node_modules/.bin:/usr/lib/node_modules/npm/node_modules/npm-lifecycle/node-gyp-bin:/home/zzak/code/testdouble.js/node_modules/.bin:/usr/lib/node_modules/npm/node_modules/npm-lifecycle/node-gyp-bin:/home/zzak/code/testdouble.js/node_modules/.bin:/home/zzak/.bin/elixir/bin:/home/zzak/work/go/bin:/home/zzak/.bin/go/bin:/home/zzak/.cargo/bin:/home/zzak/.bin:/usr/local/heroku/bin:/home/zzak/.itj/bin:/home/zzak/.gem/ruby/2.3.6/bin:/home/zzak/.rubies/ruby-2.3.6/lib/ruby/gems/2.3.0/bin:/home/zzak/.rubies/ruby-2.3.6/bin:/usr/local/bin:/usr/local/sbin:/home/zzak/.cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/zzak/.roswell/bin:/home/zzak/.local/bin
9 verbose lifecycle testdouble@3.7.0~test:example:plain-html: CWD: /home/zzak/code/testdouble.js
10 silly lifecycle testdouble@3.7.0~test:example:plain-html: Args: [ '-c', './script/run-examples plain-html' ]
11 silly lifecycle testdouble@3.7.0~test:example:plain-html: Returned: code: 254  signal: null
12 info lifecycle testdouble@3.7.0~test:example:plain-html: Failed to exec test:example:plain-html script
13 verbose stack Error: testdouble@3.7.0 test:example:plain-html: `./script/run-examples plain-html`
13 verbose stack Exit status 254
13 verbose stack     at EventEmitter.<anonymous> (/usr/lib/node_modules/npm/node_modules/npm-lifecycle/index.js:285:16)
13 verbose stack     at emitTwo (events.js:126:13)
13 verbose stack     at EventEmitter.emit (events.js:214:7)
13 verbose stack     at ChildProcess.<anonymous> (/usr/lib/node_modules/npm/node_modules/npm-lifecycle/lib/spawn.js:55:14)
13 verbose stack     at emitTwo (events.js:126:13)
13 verbose stack     at ChildProcess.emit (events.js:214:7)
13 verbose stack     at maybeClose (internal/child_process.js:925:16)
13 verbose stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:209:5)
14 verbose pkgid testdouble@3.7.0
15 verbose cwd /home/zzak/code/testdouble.js
16 verbose Linux 4.13.0-38-generic
17 verbose argv "/usr/bin/node" "/usr/lib/node_modules/npm/bin/npm-cli.js" "run" "--testdouble:build_file=dist/testdouble.js" "test:example:plain-html"
18 verbose node v8.11.1
19 verbose npm  v5.6.0
20 error code ELIFECYCLE
21 error errno 254
22 error testdouble@3.7.0 test:example:plain-html: `./script/run-examples plain-html`
22 error Exit status 254
23 error Failed at the testdouble@3.7.0 test:example:plain-html script.
23 error This is probably not a problem with npm. There is likely additional logging output above.
24 verbose exit [ 254, true ]

@searls
Copy link
Member

searls commented Apr 28, 2018

I believe if you want to make a junit report you should only pipe npm test (unit tests in a tap report) to tap-junit. npm run test:ci does a whole bunch of installs and other project tests

@@ -4,7 +4,8 @@ default_steps: &default_steps
- checkout
- run: npm install tap-junit
- run: npm install
- run: npm run test:ci | ./node_modules/tap-junit/bin/tap-junit --output test_results --name junit
- run: npm run test:ci
- run: npm run test | ./node_modules/tap-junit/bin/tap-junit --output test_results --name junit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@zzak zzak closed this Jan 16, 2021
@zzak zzak deleted the test_results branch January 16, 2021 06:51
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

Successfully merging this pull request may close these issues.

None yet

2 participants