Skip to content
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

Nightly test fixes #3208

Merged
merged 13 commits into from
May 6, 2020
Merged

Nightly test fixes #3208

merged 13 commits into from
May 6, 2020

Conversation

tafsiri
Copy link
Contributor

@tafsiri tafsiri commented May 5, 2020

This PR (incorporating Na's change which made the webgl runs sequential in nightly) passes nightly.

It primarily fixes the webworker build, but also disables the cloud benchmark integration test, which seems to use an out of date approach to building packages, (yalc link, no build-deps etc). If this was in fact working since we moved to link:// style dependencies I can put this back, else it may have to wait until we refactor those integration tests more fully.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

@tafsiri tafsiri changed the title Nightly test fixes Nightly test fixes (remember to turn off auto-nightly before merge) May 5, 2020
@tafsiri tafsiri marked this pull request as ready for review May 6, 2020 04:10
@tafsiri tafsiri requested a review from lina128 May 6, 2020 04:13
Copy link
Collaborator

@lina128 lina128 left a 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 @lina128 and @tafsiri)


scripts/run-build.sh, line 22 at r1 (raw file):

if [[ -f "$DIR/run-ci" || "$NIGHTLY" = true ]]; then
  gcloud builds submit . --config=$DIR/cloudbuild.yml \
    --substitutions _NIGHTLY=true

I see the PR has passed. Thank you Yannick! Please change this value back.

@tafsiri tafsiri changed the title Nightly test fixes (remember to turn off auto-nightly before merge) Nightly test fixes May 6, 2020
Copy link
Contributor Author

@tafsiri tafsiri left a 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 @lina128)


scripts/run-build.sh, line 22 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

I see the PR has passed. Thank you Yannick! Please change this value back.

Done. Just waiting on CI to finish, i'm getting a lot of browserstack disconnects. FYI tfjs-layers still tests firefox in non-nightly mode (that is what is disconnecting for me today). Should we switch that to just chrome?

Copy link
Contributor Author

@tafsiri tafsiri left a 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 @lina128)


scripts/run-build.sh, line 22 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

Done. Just waiting on CI to finish, i'm getting a lot of browserstack disconnects. FYI tfjs-layers still tests firefox in non-nightly mode (that is what is disconnecting for me today). Should we switch that to just chrome?

Union package also tests in firefox by default.

@tafsiri
Copy link
Contributor Author

tafsiri commented May 6, 2020

@lina128 Ready for re-review, this passes CI even though it doesn't look like it, if you click into the failing test target, I've run it and it passes. I just can't run them all at the same time without something disconnecting.

@tafsiri tafsiri requested a review from lina128 May 6, 2020 15:28
Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @lina128 and @tafsiri)


scripts/run-build.sh, line 22 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

Union package also tests in firefox by default.

I can move the firefox tests in a separate PR.

@lina128 lina128 merged commit 351f1d6 into master May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants