Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

chore: Test with GitHub Actions #78

Merged
merged 1 commit into from
Jan 22, 2020
Merged

chore: Test with GitHub Actions #78

merged 1 commit into from
Jan 22, 2020

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented Jan 22, 2020

Add a workflow to run tests with GitHub Actions.

Based on the standard Node.js starter workflow with the following changes:

  • Add pull_request event so tests are run on pull requests
  • Node.js 8.x removed
  • Node.js 13.x added
  • fail-fast: false so that all versions of Node.js are tested (without this as soon as one job fails the rest are canceled)
  • npm ci replaced with npm install as this project doesn't have a package-lock.json or shrinkwrap file committed

Tests seem very broken with tap 14 so I've downgraded to tap 12 where tests pass on Node.js 12.x and 13.x but there are still failures on 10.x (hence this being WIP). Any help with fixing those failures would be appreciated as I'm not that familiar with node-inspect.

@richardlau
Copy link
Member Author

richardlau commented Jan 22, 2020

Re. failures with tap 14, see https://github.com/richardlau/node-inspect/runs/401645634 for example failures. Most of them were timeouts. I haven't dug into them here (due to my unfamiliarity with node-inspect) but in node-report I did find that tap 14 interfered with its tests (nodejs/node-report#135) by installing signal handlers which may be the issue here if signals are being used.

The failures with tap 12 and Node.js 10 seem to be off-by-one line mismatches?
e.g. https://github.com/nodejs/node-inspect/pull/78/checks?check_run_id=402008893#step:4:47

        not ok 3 - pauses in next line of the script
          ---
          found: |-
            break in examples/break.js:1
            > 1 const x = 10;
              2 let name = 'World';
              3 name = 'Robin';
            debug> 
          pattern: 'break in examples/break.js:2'
          at:
            line: 31
            column: 9
            file: test/cli/break.test.js
            function: cli.waitForInitialBreak.then.then.then.then
          stack: |
            cli.waitForInitialBreak.then.then.then.then (test/cli/break.test.js:31:9)
          source: |
            t.match(
          ...

@richardlau
Copy link
Member Author

From #75 (comment)

It may be worth it to bump the major version and set the minimum version to 12. I don't think we'd back-port node inspect changes to node 10 given how infrequent these were in general.

@jkrems I could always leave 10.x out of this PR, which would allow a passing CI on 12 and 13 (which would be a better position than no CI at all). We could then add 10.x in a later PR if deemed necessary.

Add a workflow to run tests with GitHub Actions.
@jkrems
Copy link
Collaborator

jkrems commented Jan 22, 2020

Rebased on master and commented out 10.x. Thanks for setting this up, this is great!

@jkrems jkrems merged commit 1e1f99f into master Jan 22, 2020
@jkrems jkrems changed the title [WIP] chore: Test with GitHub Actions chore: Test with GitHub Actions Jan 22, 2020
@jkrems jkrems deleted the workflow branch January 22, 2020 15:45
@jkrems jkrems mentioned this pull request Jan 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants