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

Add VS Code configs for IDE debugging of tests #46

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

justingrant
Copy link
Contributor

This PR adds a launch.json debug configuration file that VS Code users can use for one-click IDE debugging of either Demitasse or (coming soon once #41 is merged) Test262 tests.

Demitasse test debugging "just works". You should be able to set breakpoints inside Demitasse tests and the debugger will break there.

Test262 debugging has more quirks. Notably, breakpoints don't work inside the Test262 test runner because there's no sourcemap pointing from the executing code back to the original test sources. This may not be worth solving anyways, because I suspect that generating sourcemaps for every test file would take a loooooooong time!

What does work is to place a debugger; statement inside a Test262 test that you want to debug. Execution will pause at the debugger statement and then you can inspect variables, step in/over, and set breakpoints on later code. Just remember to remove the debugger statement before committing tests! 😄

There are two Test262 debug configs:

  1. Test262 (current file) is the easiest. It just debugs the test file currently open in the editor.
  2. Test262 (subset in launch.json) debugs a subset of tests hardcoded to a glob pattern in launch.json. This is useful for repeatedly debugging the same set of multiple tests.

In addition to #41, tolerable Test262 debugging also depends on tc39/proposal-temporal#1812 which contains several fixes to the debug experience. It's not really usable without those changes.

@12wrigja
Copy link
Contributor

Just remember to remove the debugger statement before committing tests! 😄

This sounds like a useful check to put in place somewhere.

.vscode/launch.json Outdated Show resolved Hide resolved
.vscode/launch.json Outdated Show resolved Hide resolved
@justingrant
Copy link
Contributor Author

Just remember to remove the debugger statement before committing tests! 😄

This sounds like a useful check to put in place somewhere.

There's already an eslint rule warning about not using debugger. Do eslint errors cause cause CI failures? If not, then I agree this check would be good to add, but probably out of scope to this PR.

@12wrigja
Copy link
Contributor

If it already exists as an enabled lint rule, then it should break CI (as we do check for the lint returning successfully). Definitely out of scope for this PR - i'll test it shortly and send a PR to turn on the rule if it isn't enabled.

Is this ready to merge?

justingrant added a commit to justingrant/proposal-temporal that referenced this pull request Sep 10, 2021
This commit enables running Test262 in debuggers like VSCode:
* Adds a TIMEOUT environment variable (in msecs) to prevent the harnesss
  from killing the process while you're debugging it.
  Default is 10,000ms === 10 seconds. (same as existing default)
* Works around a problem where passing tests were mistakenly labelled
  as failures when run in the debugger because of Node's debug-related
  console output.

js-temporal/temporal-polyfill#46 requires this.
@justingrant
Copy link
Contributor Author

Is this ready to merge?

Yep! (Assuming you're OK with my suggestion in https://github.com/js-temporal/temporal-polyfill/pull/41/files#r706482347)

This commit adds a launch.json to enable easy IDE debugging of either
Demitasse or (coming soon in js-temporal#41) Test262 tests.

For Test262, there are two different configs:
1) Only debug the subset of tests shown in launch.json. Edit launch.json
   to debug other tests. Warning: commenting out the TESTS env var
   will debug all tests but that will take 10+ mins to run.
2) Only debug the test file currently open in the editor.
@12wrigja
Copy link
Contributor

That seems fine - let's go with the environment variable. Thanks!

@12wrigja 12wrigja merged commit edcc668 into js-temporal:main Sep 10, 2021
Ms2ger pushed a commit to tc39/proposal-temporal that referenced this pull request Sep 14, 2021
This commit enables running Test262 in debuggers like VSCode:
* Adds a TIMEOUT environment variable (in msecs) to prevent the harnesss
  from killing the process while you're debugging it.
  Default is 10,000ms === 10 seconds. (same as existing default)
* Works around a problem where passing tests were mistakenly labelled
  as failures when run in the debugger because of Node's debug-related
  console output.

js-temporal/temporal-polyfill#46 requires this.
@12wrigja
Copy link
Contributor

R.e. the debugger check: this repo's linter is already configured to catch it:

./temporal-polyfill/test/tc39/polyfill/test/Calendar/prototype/dateAdd/balance-smaller-units.js
  15:1  error  Unexpected 'debugger' statement  no-debugger

./temporal-polyfill/test/tc39/polyfill/test/PlainDate/prototype/subtract/argument-string-negative-fractional-units.js
  13:1  error  Unexpected 'debugger' statement  no-debugger

Also, this was very helpful for debugging the various test failures and narrowing down what was missing. Thank you so so much!

@justingrant
Copy link
Contributor Author

Also, this was very helpful for debugging the various test failures and narrowing down what was missing.

So glad to hear! I've been annoyed at the difficulty of debugging Temporal tests (esp. Test262) for a long time, so was good to have an excuse to make it easier.

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