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

CI: Check that test expectations are up-to-date #23201

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 17, 2024

This test will fail if the expectations are out-of-date on the main branch. This can happen due to either:

  1. Somebody forgetting to update the expectations when they land an emscripten change
  2. llvm or binaryen changes that rolled in on the auto-roller.

For now I propose that we don't make this new check "Required" and we consider this the first step towards making these updates easy and automatic.

I made this a github action rather than using CiricleCI since I anticipate using github automation here in the future (for example to suggest updates the current PR).

@sbc100 sbc100 requested review from kripken and dschuff December 17, 2024 19:11
@sbc100 sbc100 force-pushed the check_expectations branch 21 times, most recently from 7252f97 to 98bcecd Compare December 17, 2024 20:27
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@sbc100 sbc100 force-pushed the check_expectations branch from 98bcecd to 417fa17 Compare December 17, 2024 20:49
@sbc100 sbc100 marked this pull request as draft December 17, 2024 20:53
@sbc100 sbc100 force-pushed the check_expectations branch 3 times, most recently from a83b74b to 5cfa098 Compare December 17, 2024 22:08
@sbc100 sbc100 force-pushed the check_expectations branch from 5cfa098 to 22a1448 Compare December 17, 2024 22:18
@sbc100 sbc100 marked this pull request as ready for review December 17, 2024 22:28
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 17, 2024

Updated, and added some more context to the description.

@sbc100 sbc100 force-pushed the check_expectations branch from 23b202d to ea4940d Compare December 17, 2024 22:31
@kripken
Copy link
Member

kripken commented Dec 17, 2024

Sounds good, if this is just a recommendation then I am not worried about annoyance.

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 17, 2024

Sounds good, if this is just a recommendation then I am not worried about annoyance.

Its just a recommendation ... for now :)

We can chat more about the next steps we might want to build on top of this.

@sbc100 sbc100 merged commit f1639a3 into emscripten-core:main Dec 17, 2024
11 of 14 checks passed
@sbc100 sbc100 deleted the check_expectations branch December 17, 2024 22:46
hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
This test will fail if the expectations are out-of-date on the main
branch. This can happen due to either:

1. Somebody forgetting to update the expectations when they land an
emscripten change
2. llvm or binaryen changes that rolled in on the auto-roller.

For now I propose that we don't make this new check "Required" and we
consider this the first step towards making these updates easy and
automatic.

I made this a github action rather than using CiricleCI since I
anticipate using github automation here in the future (for example to
suggest updates the current PR).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants