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

test(coverage): add Test262 semantic tests #4454

Closed

Conversation

DonIsaac
Copy link
Collaborator

@DonIsaac DonIsaac commented Jul 25, 2024

What This PR Does

Adds a suite of Semantic tests that can be plugged in to existing coverage suites.

The tests seem to run correctly, but now all parser Test262 tests are failing...

Copy link

graphite-app bot commented Jul 25, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @DonIsaac and the rest of your teammates on Graphite Graphite

Copy link

codspeed-hq bot commented Jul 25, 2024

CodSpeed Performance Report

Merging #4454 will not alter performance

Comparing don/07-24-test_coverage_add_test262_semantic_tests (69ab79d) with main (b58ed80)

Summary

✅ 32 untouched benchmarks

@DonIsaac DonIsaac requested a review from Dunqing July 25, 2024 03:08
@Dunqing
Copy link
Member

Dunqing commented Jul 25, 2024

We copied a lot of tests from typescrip-eslint for testing scope/symbols in #3990. do we need this anymore?

@DonIsaac DonIsaac closed this Aug 2, 2024
@overlookmotel
Copy link
Collaborator

Despite #3990, I do still think testing scopes against Test262 too is probably a good idea, for 2 reasons:

  1. I imagine Test262 probably covers more weird edge cases (e.g. some strange stuff in sloppy mode code).
  2. chore(semantic): copy tests from typescript-eslint’s scope-manager #3990 ported the test cases, but doesn't actually check them against the typescript-eslint's snapshots (see Verify scope tree and symbols table match TypeScript #4316).

How far off were you from getting this working @DonIsaac?

@DonIsaac
Copy link
Collaborator Author

DonIsaac commented Aug 2, 2024

@overlookmotel the test infra works, but the tests need review. Not sure how to continue. @Dunqing thoughts?

@Dunqing
Copy link
Member

Dunqing commented Aug 4, 2024

I agreed to continue this work. But It's not just test262. we also need to check that the semantics in the TypeScript is correct.

@DonIsaac DonIsaac reopened this Aug 4, 2024
@Boshen
Copy link
Member

Boshen commented Aug 11, 2024

I'm going to work on this in #4790

@Boshen Boshen closed this Aug 11, 2024
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.

4 participants