Add "engines" flag to package.json

[ballercat]

Hi there, 👋

I really what you folks got going here with Buildkite Test Analytics! Noticed that we are missing support for Cypress and wanted to contribute to this project but ya'll got some issues with the e2e tests and docs. Hopefully this helps others 👍

Expected behavior

Test pass after fresh install and following CONTRIBUTING docs.

Actual behavior

End to end tests timeout if examples/jest directory is missing node_modules.

Note: Looks like this is an issue with using npm versions 6.x, not sure about 7.x vs the CI which is using 8.9.0. This change makes it work on all versions however.

Output

~/work/test-collector-javascript main*
13:07:53 ❯ npm test

> buildkite-test-collector@1.0.1 test /Users/abuldauskas/work/test-collector-javascript
> jest

 PASS  util/ci.test.js
 PASS  jest/reporter.test.js
 FAIL  e2e/jest.test.js (5.699 s)
  ● examples/jest › it posts the correct JSON

    expect(received).toMatch(expected)

    Expected pattern: /Posting to Test Analytics: ({.*})/m
    Received string:  "
    > buildkite-test-collector-jest-example@ test /Users/abuldauskas/work/test-collector-javascript/examples/jest
    > jest·
    "

      17 |     }
      18 |     exec('npm test', execOpts, (error, stdout, stderr) => {
    > 19 |       expect(stdout).toMatch(/Posting to Test Analytics: ({.*})/m);
         |                      ^
      20 |
      21 |       const jsonMatch = stdout.match(/Posting to Test Analytics: ({.*})/m)
      22 |       const json = JSON.parse(jsonMatch[1])

      at toMatch (e2e/jest.test.js:19:22)

  ● examples/jest › it posts the correct JSON

    thrown: "Exceeded timeout of 5000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

       7 |
       8 | describe('examples/jest', () => {
    >  9 |   test('it posts the correct JSON', (done) => {
         |   ^
      10 |     const execOpts = {
      11 |       cwd: path.join(__dirname, "../examples/jest"),
      12 |       env: {

      at test (e2e/jest.test.js:9:3)
      at Object.describe (e2e/jest.test.js:8:1)

Test Suites: 1 failed, 2 passed, 3 total
Tests:       1 failed, 10 passed, 11 total
Snapshots:   0 total
Time:        6.112 s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

Solution

• Test timeout after 5s because the done() that's currently in the test can never be reached. The initial expect fails the callback method, because expect throws and the rest of the code in the function can never be reached.
• Used promisify to make the code a bit easier to read and handle the error in tests without a timeout being hit
• Added a line to the readme to install example directories. Maybe consider making these not depend on an install in the future.

With this change, the test will still fail w/o node_modules in the example folder but the test suite will not timeout and only take ~2s (on my machine).

[toolmantim]

Thanks for the contribution, and sorry you had trouble getting things set up locally.

This project uses npm workspaces, introduced with npm 7. That would explain why on npm 6 it didn’t install the node_modules correctly.

Looks like we’re missing a {"engines":{"npm":">=7.0.0"} in package.json that codifies these minimum requirements.

Can you try getting started again, but with npm >= 7? Becuase if that works, I’d prefer us not merge this change but just make the above package.json change instead.

[ballercat]

Hey @toolmantim, sure no problem I can adjust the package.json in this PR / sounds reasonable to me.

Just wanted to mention that if you guys do have timeouts with these in the future due to other test runner related issues a change to the test body as in this PR would likely be something you might want to revisit!

[ballercat]

Done.

[toolmantim]

Thank you! 👍🏼👍🏼