Built-in debug mode - Githubissues

novemberborn commented 7 years ago

@Qix- in https://github.com/avajs/ava/issues/1108#issuecomment-311550068:

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.

Checkout the Issuehunt explorer to discover more funded issues.

Need some help from other developers? Add your repositories on Issuehunt to raise funds.

sabrehagen commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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.

bitjson commented 6 years ago

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 commented 6 years ago

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

novemberborn commented 6 years ago

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

krisnye commented 6 years ago

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 commented 6 years ago

This feature is really a necessity.

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

krisnye commented 6 years ago

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

novemberborn commented 6 years ago

That's what the issue is for.

LazerJesus commented 5 years ago

whats the status of this?

Qix- commented 5 years ago

Still open, clearly.

novemberborn commented 5 years ago

@FinnFrotscher would you like to give this a go?

LazerJesus commented 5 years ago

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 commented 5 years ago

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

https://github.com/avajs/ava/blob/ed7807e372f7f50d43597d966a2314bdd7a7d6ff/api.js#L269

AVA's CLI flags are here:

https://github.com/avajs/ava/blob/ed7807e372f7f50d43597d966a2314bdd7a7d6ff/lib/cli.js#L56

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

novemberborn commented 5 years ago

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

elhigu commented 5 years ago

@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 commented 5 years ago

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.

IssueHuntBot commented 5 years ago

@0maxxam0 has funded $10.00 to this issue.

Submit pull request via IssueHunt to receive this reward.
Want to contribute? Chip in to this issue via IssueHunt.
Checkout the IssueHunt Issue Explorer to see more funded issues.
Need help from developers? Add your repository on IssueHunt to raise funds.

IssueHuntBot commented 5 years ago

@issuehunt has funded $60.00 to this issue.

Submit pull request via IssueHunt to receive this reward.
Want to contribute? Chip in to this issue via IssueHunt.
Checkout the IssueHunt Issue Explorer to see more funded issues.
Need help from developers? Add your repository on IssueHunt to raise funds.

mschnee commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

geoffp commented 5 years ago

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

novemberborn commented 5 years ago

@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 commented 5 years ago

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

Gwenio commented 5 years ago

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 commented 5 years ago

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 https://github.com/avajs/ava/issues/1505#issuecomment-326961805 make that possible?

leviwheatcroft commented 5 years ago

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 commented 5 years ago

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.

avajs / ava

Built-in debug mode #1505

Recommendation