-
Notifications
You must be signed in to change notification settings - Fork 2k
[wasm] Add more device testing #3851
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
Conversation
lina128
left a comment
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.
Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @pyu10055, and @tafsiri)
tfjs-backend-wasm/src/setup_test.ts, line 227 at r1 (raw file):
{include: 'split'}, {include: 'pad ', excludes: ['complex', 'zerosLike']}, {include: 'clip', excludes: ['gradient', 'propagates NaNs']},
Hi @annxingyuan, this test fails with ios safari, the last value is not NaN, but 50, I excluded it for now. Please investigate, I will ping you the log.
annxingyuan
left a comment
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.
Reviewable status:
complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @lina128, @pyu10055, and @tafsiri)
tfjs-backend-wasm/karma.conf.js, line 108 at r1 (raw file):
browserNoActivityTimeout: 3e5, browserDisconnectTimeout: 3e5, browserDisconnectTolerance: 0,
curious why we want to set it explicitly to 0? I thought the default was 0?
tfjs-backend-wasm/scripts/test-ci.sh, line 16 at r1 (raw file):
if [ "$NIGHTLY" = true ]; then yarn run-browserstack --browsers=bs_safari_mac
just curious why you split up the runs this way?
tfjs-backend-wasm/src/setup_test.ts, line 227 at r1 (raw file):
Previously, lina128 (Na Li) wrote…
Hi @annxingyuan, this test fails with ios safari, the last value is not NaN, but 50, I excluded it for now. Please investigate, I will ping you the log.
sounds good
lina128
left a comment
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.
Reviewable status:
complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @lina128, @pyu10055, and @tafsiri)
tfjs-backend-wasm/karma.conf.js, line 108 at r1 (raw file):
Previously, annxingyuan (Ann Yuan) wrote…
curious why we want to set it explicitly to 0? I thought the default was 0?
This set of configurations are recommended settings from BrowserStack. I changed disconnect tolerance to 1 once, but changed it back because at that time disconnect happens at timeout, so reconnect doesn't help. But maybe we can try changing tolerance to 1 for wasm, because I've seen disconnect occur very early recently. For now, I prefer to leave the field here so that we can tweak it easily later.
tfjs-backend-wasm/scripts/test-ci.sh, line 16 at r1 (raw file):
Previously, annxingyuan (Ann Yuan) wrote…
just curious why you split up the runs this way?
These are split randomly. All we need is to kickoff one browser first, so that it passed the binary to Browserstack, then later browsers will all share the binary. Is there any flag you want to test too?
Added more device testing for WASM backend to nightly.
Tested nightly build in local: https://pantheon.corp.google.com/cloud-build/builds/108f3152-1162-4fbd-94df-e843b58f70f4;step=5?project=learnjs-174218
This change is