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

Debug config for tests in current file #1790

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

artwyman
Copy link
Collaborator

Adds a new VSCode debug configuration to launch unit tests from the currently open file.

This works on most but not all of the tests in the repo. The passport-server tests in particular don't work. See below for more details if you want to help fix it.

To use this:

  • Open VSCode workspace, and open the test file you want to run (wherever the cursor is determines the file to run)
  • Place breakpoints in the left margin if desired
  • Click the "Run & Debug" icon on the left to open the debug pane (left side of the screen)
  • In the dropdown at the top of the debug pane, select "TS Mocha Test (Current File)"
  • Click the play button to launch tests

The remaining issue in passport-server tests seems to have to do with type definitions for some Chai add-ons. It might occur everywhere those add-ons are used, or it might be specific to the fact that passport-server builds an app rather tha a library. Here's the error when I try to run lemonadePipeline.spec.ts:

/Users/artwyman/.nvm/versions/node/v20.6.1/bin/node ./../../../../../../node_modules/mocha/bin/_mocha -r ts-node/register --config /Users/artwyman/dev/zupass/.mocharc.js --no-timeouts --exit /Users/artwyman/dev/zupass/apps/passport-server/test/generic-issuance/pipelines/lemonade/lemonadePipeline.spec.ts

TSError: ⨯ Unable to compile TypeScript:
../../../util/mockApis.ts(34,24): error TS2339: Property 'spy' does not exist on type 'ChaiStatic'.
../../../util/mockApis.ts(35,10): error TS2339: Property 'spy' does not exist on type 'ChaiStatic'.

    at createTSError (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:859:12)
    at reportTSError (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:863:19)
    at getOutput (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1077:36)
    at Object.compile (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1433:41)
    at Module.m._compile (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1617:30)
    at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Function.Module._load (node:internal/modules/cjs/loader:938:12)
    at Module.require (node:internal/modules/cjs/loader:1115:19)
    at require (node:internal/modules/helpers:130:18)
    at Object.<anonymous> (/Users/artwyman/dev/zupass/apps/passport-server/test/util/startTestingApplication.ts:3:1)
    at Module._compile (node:internal/modules/cjs/loader:1241:14)
    at Module.m._compile (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Function.Module._load (node:internal/modules/cjs/loader:938:12)
    at Module.require (node:internal/modules/cjs/loader:1115:19)
    at require (node:internal/modules/helpers:130:18)
    at Object.<anonymous> (/Users/artwyman/dev/zupass/apps/passport-server/test/generic-issuance/pipelines/lemonade/lemonadePipeline.spec.ts:50:1)
    at Module._compile (node:internal/modules/cjs/loader:1241:14)
    at Module.m._compile (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/artwyman/dev/zupass/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Function.Module._load (node:internal/modules/cjs/loader:938:12)
    at Module.require (node:internal/modules/cjs/loader:1115:19)
    at require (node:internal/modules/helpers:130:18)
    at Object.exports.requireOrImport (/Users/artwyman/dev/zupass/node_modules/mocha/lib/nodejs/esm-utils.js:53:16)
    at async Object.exports.loadFilesAsync (/Users/artwyman/dev/zupass/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/Users/artwyman/dev/zupass/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/Users/artwyman/dev/zupass/node_modules/mocha/lib/cli/run.js:370:5)
Process exited with code 1

I welcome suggestions on how to fix this final issue, but I think this config is useful for the tests where it works.

@artwyman artwyman added the test label Jun 15, 2024
@artwyman artwyman requested a review from robknight June 15, 2024 01:29
@artwyman artwyman self-assigned this Jun 15, 2024
@ichub
Copy link
Contributor

ichub commented Jun 17, 2024

Adds a new VSCode debug configuration to launch unit tests from the currently open file.

Nice!

The remaining issue in passport-server tests seems to have to do with type definitions for some Chai add-ons. It might occur everywhere those add-ons are used, or it might be specific to the fact that passport-server builds an app rather tha a library. Here's the error when I try to run lemonadePipeline.spec.ts:

there are a few ways to proceed here, ordered by the order in which i would attempt to solve this:

  • one possible thing to do is to find/create a declaration file that compensates for these types not being there. the spy stuff isn't actually super critical for testing, so having a janky declaration solution for it is fine. I think if the error is isolated to a single file, it may even be possible to augment the type in-line in that file. Or use as any.
  • figure out the difference between the way this code is run/compiled between the vs-code and non-vs-code runner. as far as i understand the CLI test invocation does do typechecking, which succeeds, so there must be some difference which can be accounted for.
  • turn off type-checking altogether somehow for the tests when they are run using the vs-code debugger. this option is not super great, but would not lead to errors making it into main as the CI runs all the tests anyways, and typechecks them as well.
  • in the worst case scenario we could even remove the spy dependency. i added it in a fit of passion last summer, and i'm not sure how much value it provides.

@artwyman
Copy link
Collaborator Author

Note it's not just spy. With other files I saw similar errors about checking for promise rejection, which I believe is tied to the chai-as-promised library.

  • figure out the difference between the way this code is run/compiled between the vs-code and non-vs-code runner. as far as i understand the CLI test invocation does do typechecking, which succeeds, so there must be some difference which can be accounted for.

This is what I'd like to do, but I feel like I need to understand the TypeScript tooling better to do so. @robknight might have more insight on this. Ideally VSCode would be able to run exactly the same command as yarn test runs from the command line (which is ts-mocha...). Using the _mocha executable here with the -r ts-node/register is a trick I found on Stack Overflow to make a consistent command line which mostly works, with the issue I called out here. Importantly, it works with cwd set to ${fileDirname}.

I actually can make a working config for the same file which calls ts-mocha directly (as ${workspaceFolder}/node_modules/ts-mocha/bin/ts-mocha). The problem is that it seems to be highly sensitive to its working directory, where it expects to find tsconfig.json. If cwd is set to the package directory (location of package.json) then it works. If it's set to anything else, it fails to find tsconfig.json. The package directory isn't something VSCode has a variable for, and it also isn't always the same number of levels up from the location of a test file, so I failed to find something which works. Things I thought to try:

  • ts-mocha takes a --package arg, but it points to a package file, and doesn't seem to affect tsconfig.
  • Even if it did, I have no way to construct a path to the package file in a way which works for all files.
  • There are extensions which could allow me to execute a command inside of a variable expansion, but that seems like a big can of worms to open in order to solve this.

I'll put this PR in draft and upload my alternative version for discusssion.

@artwyman artwyman marked this pull request as draft June 18, 2024 01:45
@artwyman
Copy link
Collaborator Author

Updates:

  • Running commands inside variables is actually built-in, not an extension. However I couldn't find any useful built-in commands. The ability to run a shell command takes an extension.
  • I notice that our workspace root has tsconfig.cjs.json and tsconfig.esm.json but no tsconfig.json. That's a dir I can point to with a VSCode variable. I wonder if it's possible to put a tsconfig.json file there which will meet the needs of unit tests without messing things up for other builds. I don't understand the interactions of different tsconfig files well enough to be sure.

@robknight
Copy link
Member

Updates:

* Running commands inside variables is actually built-in, not an extension.  However I couldn't find any useful built-in commands.  The ability to run a shell command takes an extension.

* I notice that our workspace root has `tsconfig.cjs.json` and `tsconfig.esm.json` but no `tsconfig.json`.  That's a dir I can point to with a VSCode variable.  I wonder if it's possible to put a `tsconfig.json` file there which will meet the needs of unit tests without messing things up for other builds.  I don't understand the interactions of different tsconfig files well enough to be sure.

I think that would be OK.

@artwyman
Copy link
Collaborator Author

I think that would be OK.

How would I go about setting that up and making sure it stays up-to-date. Do I just need to manually fill in the references normally filled in by yarn fix-references? Is the presence of that file ever going to confuse other parts of the build?

I'm annoyed that I can't even put a comment in that file saying what it's for, because JSON doesn't allow that. If the compiler ignores unknown fields, I may try putting a comment in a string in the JSON.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants