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

Robust testing for scope analysis #2947

Open
hyf0 opened this issue Apr 12, 2024 · 0 comments
Open

Robust testing for scope analysis #2947

hyf0 opened this issue Apr 12, 2024 · 0 comments
Assignees
Labels
C-enhancement Category - New feature or request

Comments

@hyf0
Copy link
Contributor

hyf0 commented Apr 12, 2024

This issue is follow-up of #1227 and #2498.

Status quo

oxc currently rely on test262 and manual testing.

test262:
https://github.com/oxc-project/oxc/blob/1519b9000bcdc7c43c79c8b26e194a13f7df1f1e/crates/oxc_semantic/tests/scopes.rs

manual testing:

fn are_all_identifiers_resolved(semantic: &oxc_semantic::Semantic<'_>) -> bool {
use oxc_ast::AstKind;
use oxc_semantic::AstNode;
let ast_nodes = semantic.nodes();
let has_non_resolved = ast_nodes.iter().any(|node| {
match node.kind() {
AstKind::BindingIdentifier(id) => {
let mut parents = ast_nodes.iter_parents(node.id()).map(AstNode::kind);
parents.next(); // Exclude BindingIdentifier itself
if let (Some(AstKind::Function(_)), Some(AstKind::IfStatement(_))) =
(parents.next(), parents.next())
{
return false;
}
id.symbol_id.get().is_none()
}
AstKind::IdentifierReference(ref_id) => ref_id.reference_id.get().is_none(),
_ => false,
}
});
!has_non_resolved
}

  • test262 only ensure the all indentifier are resolved, doesn't ensure correctness.
  • Manual testing ensure correctness but only have a few cases.

Problems

With the above summary, the current scope analysis is running in half dark.

  • Can't not detect effects/changes. If someone changes scope analysis code, we could only rely on the few manual tests to ensure the correctness.
  • Dx of writing Manual tests is bad. The API could handle simple cases well. But we have to admit that, as much as we want to try make the test case simple, there's a bigger chance we need to handle complicated code. Writing manual tests for them seems impossible with current test API.

Solutions

  1. We assume the current scope analysis behaviors are correct and visualize them in snapshot testing.
  2. Visualize scope information of tests262 and also manual tests.

If someone change scope analysis code, we could review if it is correct by review changes of snapshot.

@hyf0 hyf0 added the C-enhancement Category - New feature or request label Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category - New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants