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

Run full test suite for non-PR builds #4183

Closed
wants to merge 1 commit into from

Conversation

lahma
Copy link
Contributor

@lahma lahma commented Jul 31, 2024

I'm proposing to run full suite of tests when running against main branch. This will give full listing of currently unsupported tests by engine. Also updating to latest NPM dependencies.

Test will still be green on CI like for PRs when errors are present, see 1ebd34b for original reasoning not erroring.

I would argue that it would be better if the PRs would fail and maintainers would need to read the actual error output to figure out whether problems are correct. The test262-harness has a flag for this --error-for-failures which would break PR builds if there's a problem. Of course this would create noise for unsupported features per engine.

* update NPM dependencies
@lahma lahma requested a review from a team as a code owner July 31, 2024 07:04
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

seems fine conceptually

@ptomato
Copy link
Contributor

ptomato commented Aug 5, 2024

See also #4118. I think that would be better than having the PRs fail, because most of the PRs submitted to Test262 are for not-yet-supported features.

I wouldn't be opposed to running the whole suite on main, but two things:

  1. How is it different from test262.fyi? The website seems like it puts that information in a form that's easier to consume than clicking through to CircleCI and scrolling through the log.
  2. I'd like to avoid adding more CircleCI stuff, as CircleCI seems like it's been less reliable than GitHub Actions. How about having this as a separate GitHub action, if we decide to have it?

@lahma
Copy link
Contributor Author

lahma commented Aug 18, 2024

I was not aware of https://test262.fyi/ , thank you for pointing that out. After considering this again, it's such two-edged sword and problem. The linked idea of commented result table sounds like a better approach and would bring possible problems to the front and center. I'm closing this in light of this new information.

@lahma lahma closed this Aug 18, 2024
@lahma lahma deleted the full-suite-for-main branch August 18, 2024 15:34
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.

3 participants