-
Notifications
You must be signed in to change notification settings - Fork 445
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
Updating CI files #132
Updating CI files #132
Conversation
So upgrading wrtc (to latest 0.0.63) fixes issues on linux, but breaks thing on macOS... However, even with old version, macOS is failing in timeout in one test (even when tests timeout is set to 1 minute [Edit: I see now that that test is overriding the timeout amount...]), and miss-match in number of peers in another...
|
package.json
Outdated
@@ -6,7 +6,7 @@ | |||
"scripts": { | |||
"lint": "aegir lint", | |||
"build": "aegir build", | |||
"test": "aegir test -t node -t browser", | |||
"test": "aegir test -t node -t browser --timeout 60000", |
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 think we want timeouts per tests instead of global timeouts - should gives us a better idea of what's slow. That's what we've been leaning towards at least.
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.
Thanks! I think setting short timeouts and then having logic for retrying until success is the way to go, then we don't have to fiddle with timeouts.
However, this change here is just made to confirm if things are really broken, or tests are just failing because of timeouts.
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.
Btw, "retrying until success" would look something like this:
js-libp2p/test/nodejs-bundle/tcp+websockets+webrtc-star.js
Lines 197 to 228 in 10357a3
it('nodeAll.dial nodeWStar using PeerInfo', function (done) { | |
nodeAll.dial(nodeWStar.peerInfo, (err) => { | |
expect(err).to.not.exist() | |
const numberOfTries = 10 | |
const pauseBetweenTries = 10 | |
let currentTry = 1 | |
function check () { | |
console.log('checking') | |
const peersAll = nodeAll.peerBook.getAll() | |
const peersWStar = nodeWStar.peerBook.getAll() | |
try { | |
expect(Object.keys(peersAll)).to.have.length(3) | |
expect(Object.keys(peersWStar)).to.have.length(1) | |
expect(Object.keys(nodeAll.swarm.muxedConns)).to.have.length(1) | |
console.log('everything ok') | |
done() | |
} catch (err) { | |
if (currentTry >= numberOfTries) { | |
console.log('done trying') | |
done(err) | |
} else { | |
currentTry = currentTry + 1 | |
setTimeout(check, pauseBetweenTries) | |
} | |
} | |
} | |
check() | |
}) | |
}) |
10357a3
to
ab153ed
Compare
Leaving this for today, back at it tomorrow. Status: CircleCI is failing because the machine has to be upgraged, I have not yet done this as it would mean existing PRs and PRs in-between the change is made, and this very PR is merged, would not pass. Jenkins is having troubles with dialing on windows, ( @richardschneider oh windows god, any ideas? ), log is here:
TravisCI seems fine. |
c25791c
to
b08506f
Compare
This commit updates all CI scripts to the latest version
b08506f
to
ccac014
Compare
@victorbjelkholm going to merge this PR to have the latest configs and timeouts. Note that:
Mind checking why separately ? |
## [6.0.2](libp2p/js-libp2p-tcp@v6.0.1...v6.0.2) (2022-11-17) ### Bug Fixes * update metric names to follow prometheus naming guide ([libp2p#228](libp2p/js-libp2p-tcp#228)) ([24c5b37](libp2p/js-libp2p-tcp@24c5b37)) ### Trivial Changes * add test for filtering unix socket address ([libp2p#229](libp2p/js-libp2p-tcp#229)) ([efcfbb2](libp2p/js-libp2p-tcp@efcfbb2)), closes [libp2p#132](libp2p/js-libp2p-tcp#132)
This commit updates all CI scripts to the latest version