-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: Re-enable interface tests and get them succeeding #83
Changes from 82 commits
3bf928e
07fed40
58243e5
8ce3abb
27db517
2a282dc
0c84c21
5b9c92d
c8b95e7
aa3948d
f8598e1
b9e5325
0c182ff
aa1c7e6
63916fe
b6f7c01
e22f89c
bd67b63
011268f
298f534
e370e56
c002718
8ab1d39
c57ecc4
47b0a5a
1efca00
8765872
731b9e9
16cc34f
e0d0f7e
f29d6b8
e0f4fb1
720791a
a3daf1f
2723b6e
6185634
e0a2eea
3a76268
8623205
33d1c6b
7379ba0
31e5ad6
60e38a2
c8819db
e70563a
d8efa60
01f8282
1e98feb
36c5f60
3c47b9b
4cbe1d0
936156f
8c08e5d
8e8538e
9b7f879
4fc3866
268f69d
fb4a1b4
064dbeb
5169bb4
5c11c55
f18ecbb
16b6925
e0ee6e2
7fe16b7
089f71d
80b1a9c
cc0941f
0b5c93e
37be08a
8f5883e
e2e89cf
bcf3c79
2a8ce3a
652d3c8
d1214c6
a9b2b71
cb2566a
0be613a
42773b5
65d42f1
b3da286
6fa916a
7a11168
e2f3aa0
26c1504
059ce51
6c27e9d
f39eb81
70058a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -27,74 +27,35 @@ jobs: | |||||
strategy: | ||||||
matrix: | ||||||
os: [windows-latest, ubuntu-latest, macos-latest] | ||||||
node: [16] | ||||||
fail-fast: true | ||||||
fail-fast: false | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- uses: actions/setup-node@v3 | ||||||
with: | ||||||
node-version: ${{ matrix.node }} | ||||||
node-version: lts/* | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This project is managed via github.com/protocol/.github (config). Does this mean the changes to this file from this PR will be overwritten by https://github.com/protocol/.github/blob/master/templates/.github/workflows/js-test-and-release.yml the next time CI sync runs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it will be overwritten:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @galargh can you confirm? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's going to get overwritten. Whenever https://github.com/protocol/.github/actions/workflows/dispatch.yml runs, it updates uCI files in all the repositories where it's configured (if needed of course). That's why files managed by uCI have this header: js-kubo-rpc-client/.github/workflows/js-test-and-release.yml Lines 1 to 2 in b3da286
Here's an automated PR that wants to update the workflow here: #85 (it would have been automerged if only If there's a change we want to introduce that would benefit all JS repos participating in uCI, then this would be the place to do it: https://github.com/protocol/.github/blob/master/templates/.github/workflows/js-test-and-release.yml Side note: the way Unified CI provisions files is different from how GitHub Management does it. For example, if we were to modify https://github.com/ipfs/js-kubo-rpc-client/blob/master/.github/workflows/stale.yml which is managed here https://github.com/ipfs/github-mgmt/blob/eed32757b57354659c07d738d36ecfbc25a033ef/github/ipfs.yml#L4589-L4591, then the next sync would have noticed that the two diverged and would update the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if a project needs to disable node LTS or other node versions for some reason, what is the best way to accomplish that? Can we make the unified CI workflow dynamic with a default to LTS? I'm not sure the best way to accomplish this but it would allow individual projects to update/override when necessary. Without being overwritten. |
||||||
- uses: ipfs/aegir/actions/cache-node-modules@master | ||||||
- run: npm run --if-present test:node | ||||||
- uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # v3.1.0 | ||||||
with: | ||||||
flags: node | ||||||
|
||||||
test-chrome: | ||||||
needs: check | ||||||
runs-on: ubuntu-latest | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- uses: actions/setup-node@v3 | ||||||
with: | ||||||
node-version: lts/* | ||||||
- uses: ipfs/aegir/actions/cache-node-modules@master | ||||||
- run: npm run --if-present test:chrome | ||||||
- uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # v3.1.0 | ||||||
with: | ||||||
flags: chrome | ||||||
|
||||||
test-chrome-webworker: | ||||||
needs: check | ||||||
runs-on: ubuntu-latest | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- uses: actions/setup-node@v3 | ||||||
with: | ||||||
node-version: lts/* | ||||||
- uses: ipfs/aegir/actions/cache-node-modules@master | ||||||
- run: npm run --if-present test:chrome-webworker | ||||||
- uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # v3.1.0 | ||||||
with: | ||||||
flags: chrome-webworker | ||||||
|
||||||
test-firefox: | ||||||
needs: check | ||||||
runs-on: ubuntu-latest | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- uses: actions/setup-node@v3 | ||||||
with: | ||||||
node-version: lts/* | ||||||
- uses: ipfs/aegir/actions/cache-node-modules@master | ||||||
- run: npm run --if-present test:firefox | ||||||
- uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # v3.1.0 | ||||||
with: | ||||||
flags: firefox | ||||||
|
||||||
test-firefox-webworker: | ||||||
test-non-node-target: | ||||||
needs: check | ||||||
runs-on: ubuntu-latest | ||||||
strategy: | ||||||
matrix: | ||||||
target: [chrome, chrome-webworker, firefox, firefox-webworker] | ||||||
fail-fast: false | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- uses: actions/setup-node@v3 | ||||||
with: | ||||||
node-version: lts/* | ||||||
- uses: ipfs/aegir/actions/cache-node-modules@master | ||||||
- run: npm run --if-present test:firefox-webworker | ||||||
- run: npm run --if-present test:${{ matrix.target }} -- --bail=false | ||||||
- uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # v3.1.0 | ||||||
with: | ||||||
flags: firefox-webworker | ||||||
flags: ${{ matrix.target }} | ||||||
|
||||||
test-electron-main: | ||||||
needs: check | ||||||
|
@@ -125,7 +86,7 @@ jobs: | |||||
flags: electron-renderer | ||||||
|
||||||
release: | ||||||
needs: [test-node, test-chrome, test-chrome-webworker, test-firefox, test-firefox-webworker, test-electron-main, test-electron-renderer] | ||||||
needs: [test-node, test-non-node-target, test-electron-main, test-electron-renderer] | ||||||
runs-on: ubuntu-latest | ||||||
if: github.event_name == 'push' && github.ref == 'refs/heads/master' # with #262 - 'refs/heads/${{{ github.default_branch }}}' | ||||||
steps: | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: when we run Node with this, does it impact the runtime of tests, or is it run in a separate process?
(if it is the same process, it is a problem: we should be running tests with default Node LTS and no special parameters)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I fully understand your question, but I'll give some context around this:
opt-in
flag--experimental-fetch
, but in 18 was switched toopt-out
with--no-experimental-fetch
. Basically, any discrepancies between node-fetch and undici's fetch will need to be handled if we don't override global fetch in node with node-fetch.test/node/agent.js
test.node run test
aegir test
I think item 2 above tells us the answer to your question is that this is a separate process.