Skip to content

Built-in debug mode #1505

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

Closed
novemberborn opened this issue Sep 3, 2017 · 36 comments · Fixed by #2316
Closed

Built-in debug mode #1505

novemberborn opened this issue Sep 3, 2017 · 36 comments · Fixed by #2316
Assignees
Milestone

Comments

@novemberborn
Copy link
Member

novemberborn commented Sep 3, 2017

Issuehunt badges

@Qix- in #1108 (comment):

Preferably AVA could have a command such as ava --inspect that would simply run node --inspect and then issue debugger; right before it runs the tests.

There is a $70.00 open bounty on this issue. Add more on Issuehunt.

@sabrehagen
Copy link

I came here to make the very same request. If the parent ava process is passed --inspect=host:port it could pass it to the child process test runners. I use remote debugging which requires --inspect=0.0.0.0:9229 but the docs only support --inspect.

@novemberborn
Copy link
Member Author

If the parent ava process is passed --inspect=host:port it could pass it to the child process test runners.

We'd have to change the concurrency to 1 when a specific port is provided, otherwise all workers will try to bind to the same port.

Besides that, perhaps a --debug option could be used which picks an available port and uses it for all the workers, running only a single worker at a time.

@sabrehagen
Copy link

We'd have to change the concurrency to 1 when a specific port is provided

Yes, this sounds ideal. Can you reference the location in the code where this change would need to be made and one of us open source contributors can get to work on it?

@novemberborn
Copy link
Member Author

Can you reference the location in the code where this change would need to be made and one of us open source contributors can get to work on it?

Sure thing. Concurrency is determined here: https://github.com/avajs/ava/blob/1df502df8cba18f92afcbaed730d8c724687ca45/api.js#L164:L174

We massage the --inspect and --debug flags here:

https://github.com/avajs/ava/blob/1df502df8cba18f92afcbaed730d8c724687ca45/api.js#L177:L221

Looking at that I think these are node-level arguments, e.g. node --inspect node_modules/.bin/ava. Without having thought about the full ramifications, it'd be good if we could support ava --inspect instead.

@bitjson
Copy link
Contributor

bitjson commented Mar 6, 2018

Another option here I'd love to see considered: a standard-feeling node --inspect workflow.

One of my favorite aspects about AVA is the apparent lack of "magic" – test files look normal, and testing infrastructure must be imported rather than magically appearing in globals. Even when AVA performs some "magic" for Babel compilation, users interact with it like normal javascript, so there's nothing new to learn/configure. I'd love to see this philosophy carry over to debugging.

It would seem much more intuitive for me to be able to debug AVA tests using the same configuration as my other work. Rather than configuring my tooling to use an ava --inspect mode, it would be great if I could do what I do everywhere else: node --inspect compiled/test.js. Hopefully this would also require less setup/customization, since a lot of debugging tooling is designed/documented to fit into this workflow.

E.g. It's pretty difficult to get debugging of AVA tests working in VSCode with TypeScript right now. I'm working from this recipe. And it doesn't seem possible to get "one-click" debugging without changes to either AVA or VSCode. (I'm still looking for workarounds in typescript-starter@next.)

Recommendation

When executing a test outside of the CLI, AVA could look for either:

  • an environment variable, like AVA_DEBUG_MODE
  • if there is an active inspector

If found, AVA can setup the infrastructure currently in ./profile.js, and allow the user to debug that test file as expected.

Of course, if a "debug mode" isn't detected, AVA can still log the Test files must be run with the AVA CLI message.

Related: #1495

@bitjson
Copy link
Contributor

bitjson commented Mar 15, 2018

To provide an example of where diverging from the the node --inspect workflow causes incompatibility in tooling, here's typescript-starter's .vscode/launch.json: https://github.com/bitjson/typescript-starter/blob/master/.vscode/launch.json#L52

@novemberborn
Copy link
Member Author

We should support the modern --inspect flags, passable to AVA directly. That should make it easier to debug with WebStorm too (see #1787).

@novemberborn
Copy link
Member Author

Have folks seen https://github.com/GoogleChromeLabs/ndb? Would be great to make AVA work with that out of the box.

@krisnye
Copy link

krisnye commented Oct 5, 2018

This feature is really a necessity. When launched with --inspect it should run tests one at a time and break on the first debugger statement.

@novemberborn
Copy link
Member Author

This feature is really a necessity.

@krisnye I agree! Would you like to help us make it a reality?

@krisnye
Copy link

krisnye commented Oct 8, 2018

Maybe, you'll take a pull request with it?

@novemberborn
Copy link
Member Author

That's what the issue is for.

@LazerJesus
Copy link

whats the status of this?

@Qix-
Copy link
Contributor

Qix- commented Jan 12, 2019

Still open, clearly.

@novemberborn
Copy link
Member Author

@FinnFrotscher would you like to give this a go?

@LazerJesus
Copy link

I have switched to Mocha to get around this problem, but the solution is not satisfactory since it comes with another set of drawbacks.
Could you point me to the right place in Ava? I have not looked much under the hood yet.

@novemberborn
Copy link
Member Author

@FinnFrotscher the Node.js flags passed to the worker processes are controlled here:

ava/api.js

Line 269 in ed7807e

_computeForkExecArgv() {

AVA's CLI flags are here:

ava/lib/cli.js

Line 56 in ed7807e

flags: {

There's more stuff in api.js around the worker concurrency.

@novemberborn
Copy link
Member Author

The V8 inspector is available programatically, which could be another interesting approach: https://nodejs.org/api/inspector.html

@elhigu
Copy link

elhigu commented Apr 17, 2019

@novemberborn Actually that seems to work really nice. No need to setup separate test runner scripts to package.json to run in serial mode etc. and when ever I need to stop execution to wait for debugger I can call in test this:

function waitForDebugger() {
  console.log('Waiting for debugger to connect before continue...');
  const inspector = require('inspector');
  inspector.open(9229, 'localhost', true);
  debugger; // for some reasons breakpoints doesnt work before this
}

test(`should be able to send file`, async t => {
  // ....
  waitForDebugger();
  // ....
});

@langri-sha
Copy link
Contributor

It would really be nice if we could just pass a convenient --inspect.

I've read the guide https://github.com/avajs/ava/blob/master/docs/recipes/debugging-with-chrome-devtools.md here and found it requires a bit of muscle power to point to an actual test module and think this would be just the thing to take that work away.

@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 9, 2019
@IssueHuntBot
Copy link

@0maxxam0 has funded $10.00 to this issue.


@IssueHuntBot
Copy link

@IssueHunt has funded $60.00 to this issue.


@mschnee
Copy link

mschnee commented Jul 11, 2019

Food for thought, since we've been dancing around it but haven't stated it explicitly:

Generally, when we are actively debugging a test, we are actively debugging a single test, not all of our tests. In most cases, we are using the test as an entry point to debug specific code. Sometimes we're debugging the test itself.

We can already tell ava to run a specific file, and even a specific test in that file with --match, even though the implementation is more like a filter on a set versus an specific runner, but I would expect that (for vscode users) that a debug-test launcher would invoke ava --inspect -s ${file}

Unfortunately, for now, because the official documentation says to use profile.js, and profile.js is busted with the following error after every single yarn or npm i, I've switched a number of my projects to mocha or jest

Error: ENOENT: no such file or directory, open '/home/me/path/to/projectsi/node_modules/.cache/ava/e3c6f559ac2d3846dec13354d0dfced6.js.map.2355219466'

@langri-sha
Copy link
Contributor

Unfortunately, for now, because the official documentation says to use profile.js, and profile.js is busted with the following error after every single yarn or npm i, I've switched a number of my projects to mocha or jest

Yes, this is a nuisance. @mschnee as a workaround you can add a run script such as:

{
  "script": {
    "ava:debug": "mkdir -p node_modules/.cache/ava && node inspect node_modules/ava/profile.js"
  }
}

@novemberborn
Copy link
Member Author

Yes, this is a nuisance. @mschnee as a workaround you can add a run script such as:

{
  "script": {
    "ava:debug": "mkdir -p node_modules/.cache/ava && node inspect node_modules/ava/profile.js"
  }
}

This looks like an honest oversight in profile.js! @langri-sha would you be interested in opening a PR that ensures the directory gets created?


In lieu of debug mode being added to AVA itself, have any of you tried using ndb? You can launch it (npx ndb .) and then run an npm script from your package.json. I'm able to set breakpoints in test files and have the debugger stop execution. I wonder if that's sufficient for now, allowing us to remove the profile.js approach.

@mschnee
Copy link

mschnee commented Jul 23, 2019

FYI - that cache directory 'fix' will not work in native monorepositories. The last thing you want to do is mkdir -p ${workspaceDir}/packages/my-repo/node_modules/...anything because it will break node's module resolution - require() will find ${workspaceDir}/packages/my-repo/node_modules instead of ${workspaceDir}/node_modules. This isn't the only offender, I have a blacklist of modules I cannot use in certain projects because they create non-configurable directories for their own use in what should IMHO be protected space.

Ava/profile.js can not be used at all for these subprojects.

I would much rather see support in ava for explicitly running (and debugging/inspecting) a single test directly (a specific test in a specific file, or just a specific file). This would additionally enable a host of other things, like the possibility of creating a TestExplorer plugin for ava, better UI integration, etc.

@novemberborn
Copy link
Member Author

FYI - that cache directory 'fix' will not work in native monorepositories. The last thing you want to do is mkdir -p ${workspaceDir}/packages/my-repo/node_modules/...anything because it will break node's module resolution - require() will find ${workspaceDir}/packages/my-repo/node_modules instead of ${workspaceDir}/node_modules. This isn't the only offender, I have a blacklist of modules I cannot use in certain projects because they create non-configurable directories for their own use in what should IMHO be protected space.

I'm pretty sure we hook into the module loading and retrieve the file contents from the cache directory, without impacting the require paths. If this is causing problems in monorepos could you open a separate issue to discuss?

@geoffp
Copy link

geoffp commented Aug 19, 2019

@novemberborn the programmatic use of inspector.open() works great for me. Do we think it's worth adding that to the official docs as the recommendation for debugging in Chrome/DevTools, since what's currently there seems not to work...?

@novemberborn
Copy link
Member Author

@geoffp oh that's great! I think for a start let's make sure we have that covered in a recipe.

@geoffp
Copy link

geoffp commented Aug 20, 2019

@novemberborn okay. Let me know if you want a hand writing that up.

@novemberborn
Copy link
Member Author

@geoffp yes I haven't used this myself. Perhaps you could kick this off? Always better coming from somebody who's used the thing.

@geoffp
Copy link

geoffp commented Aug 22, 2019

@novemberborn Sure thing. I'll try to carve out the time in the coming week!

@Gwenio
Copy link

Gwenio commented Aug 28, 2019

Here are some thoughts I have on this implementing AVA test debugging for the VSCode AVA Test Explorer plugin I wrote:

  • When a worker is run with an assigned inspect port, an event needs to be generated so the port can be captured.
  • Whether or not a run will use debugging should be controlled by the parameters to Api.run(files, runtimeOptions).
  • Better yet if all the configuration of an Api's debugging was passed to run, to work towards the Api objects being reusable for multiple runs with different options.

If anyone is curious, the work around to make test debugging work in my plugin is:

const api = new Api({ /* options */})
if (port) { // port is previously selected with the get-port module.
	const original = api._computeForkExecArgv.bind(api)
	api._computeForkExecArgv = async function(): Promise<string[]> { // typescript
		const base = await original()
		const filtered = base.filter((a): boolean => !a.startsWith('--inspect'))
		return filtered.concat(`--inspect-brk=${port}`) // VSCode always expects the debug target to break.
	}
}

The reporter used with this triggers a debug session for the selected port when it's startRun method is called.

@novemberborn
Copy link
Member Author

Hi @Gwenio, I saw your post on Spectrum and have been meaning to look that up. Very nice!

We're currently not interested in supporting programmatic use of the Api, but your use case should be possible via the CLI. Would addressing #1505 (comment) make that possible?

@leviwheatcroft
Copy link

If I understand correctly, the current best workaround is to use node inspector programatically.

I couldn't get the inspect-process thing to work, some sort of weirdness with process.env I think.

So in the short term we're going to update the debugging-with-chrome-devtools recipe right?

I haven't really played around with it much, but I think it's as easy as this:

const test = require('ava')
const inspector = require('inspector')
inspector.open()
inspector.waitForDebugger() // only available in node ^12.7.0

test('my passing test', t => {
        debugger
	t.pass();
});

Probably not worth including in the docs here, but as an aside waiting for the debugger to connect seems somewhat broken in node 12.1.0. Rather than inspector.open(true) you need inspector.open(undefined, undefined, true), even though host & port are supposed to be optional

@novemberborn
Copy link
Member Author

So in the short term we're going to update the debugging-with-chrome-devtools recipe right?

That'd be great.

as an aside waiting for the debugger to connect seems somewhat broken in node 12.1.0.

That's a really old version of Node.js 12 though.

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

Successfully merging a pull request may close this issue.