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

Expose test file path within the test process #1976

Closed
coreyfarrell opened this issue Nov 5, 2018 · 11 comments · Fixed by #1977
Closed

Expose test file path within the test process #1976

coreyfarrell opened this issue Nov 5, 2018 · 11 comments · Fixed by #1977

Comments

@coreyfarrell
Copy link
Contributor

Description

I need the ability to save test artifacts similar to how t.snapshot creates .md and .snap files. This requires knowing the filename of the actual test file being executed to be available - I assume via t but I'm open to any suggestions. I tried finding what I needed though process but process.argv[2] is undefined. Even if the test filename were available from process I think it would be better to get it from t.

Test Source

First I tried using t.snapshot on image captures from the browser:
https://github.com/cfware/rollup-webapp/blob/be9505c214bc2efead7e1467c322bc587bd46aa1/test/helpers/selenium.js#L49-L55
https://github.com/cfware/rollup-webapp/blob/be9505c214bc2efead7e1467c322bc587bd46aa1/test/helpers/selenium.js#L86-L92
https://github.com/cfware/rollup-webapp/blob/be9505c214bc2efead7e1467c322bc587bd46aa1/test/helpers/pages.js#L4-L10

Unfortunately different test environments cause the browser to render fonts differently so Travis-CI cannot match snapshot PNG's generated in my local testing.

My new plan is to create a function t.context.saveImage(ele, imageID) to create files such as test/captures/firefox.js-app.html-imageID.png so they can be manually reviewed. Sorry I don't have a simple example but if test/firefox.js were to create artifacts directly I would be able to just use __filename.

Environment

Node.js v10.11.0
linux 4.18.10-200.fc28.x86_64
ava 1.0.0-rc.1
npm 6.4.1
@novemberborn novemberborn changed the title Please expose Runner.file via ExecutionContext. Expose test file path within the test process Nov 6, 2018
@novemberborn
Copy link
Member

Yea, I've had a similar use case, but for loading fixtures after resolving a source map.

Not sure about attaching it to the per-test execution context. Maybe it can be available through the 'ava' import somehow. import { filePath } from 'ava' and test.filePath?

@coreyfarrell
Copy link
Contributor Author

Yes that seems like a better place for it to be.

@sholladay
Copy link

sholladay commented Nov 13, 2018

I don't really understand this. You can already get the filepath of the test (or any other file being run in Node) using __filename (as you mentioned). This is way better since it's not AVA-specific.

Soon you will be able to use import.meta.url, which is standardized at an even higher level. I believe this one already works but is sitting behind an experimental flag in Babel for now.

@coreyfarrell
Copy link
Contributor Author

@sholladay __filename is only correct from the test file itself. The point of this bug is to be able to know the test path from helper sources - without needing to pass __filename to helper functions. For the same reason import.meta.url will not solve this issue.

@sholladay
Copy link

sholladay commented Nov 13, 2018

I see. I think I didn't catch onto that because you would also have to pass t to the helper, so it doesn't seem any better/worse in that regard.

Wouldn't import { filePath } from 'ava be tricky to implement since it has to be statically defined? Or is AVA compiled per-process? Should be interesting. :)

@novemberborn
Copy link
Member

Wouldn't import { filePath } from 'ava be tricky to implement since it has to be statically defined? Or is AVA compiled per-process? Should be interesting. :)

AVA loads the test file, so it can set that up in advance.

@coreyfarrell
Copy link
Contributor Author

@sholladay I've submitted a patch for the functionality in #1977, was pretty straight forward.

novemberborn pushed a commit that referenced this issue Jan 13, 2019
This adds `test.meta.file` which exposes the filename of the test being run by AVA.

Fixes #1976
@coreyfarrell
Copy link
Contributor Author

@novemberborn could you reopen this? I'm experimenting with some potentially advanced features of nyc. Specifically if we run nyc --require ./reconfigure-nyc.js ava this will cause nyc to wrap each node sub-process and ./reconfigure-nyc.js will be required. I'd like to experiment with features such as restricting the nyc include option based on which test is currently running. Pseudo-code example:

if (currentTest === 'test/feature1.js') {
  nycConfig.include = ['lib/feature1.js']; // only instrument the current test target
}

Unfortunately these --require's are run before require('ava').meta.file can be used. My hope is that we could have process.argv include the current test filename.

@novemberborn
Copy link
Member

@coreyfarrell I think you're running into something else. #2035 changed how the test file is passed to the worker process, so it's no longer available through process.argv no matter how early you hook it.

We'll also, at some point, support different styles of workers, see #2011. Of course it'd be fine if not all of those modes can be hooked into by nyc, that just means AVA should incorporate nyc directly.

The previous discussions have all focused on how this is exposed through AVA itself. I think for your use case we need something outside of AVA. Perhaps an environment variable: AVA_TEST_FILE_FOR_DEDICATED_WORKER_PROCESS? Excuse the verbosity but hopefully it makes clear that that variable may not be available in other execution modes.

@coreyfarrell
Copy link
Contributor Author

So you're suggesting that ava would set process.env.AVA_TEST_FILE_FOR_DEDICATED_WORKER_PROCESS='test/feature1.js' that way an nyc --require hook could see that and perform desired manipulations of the configuration?

I'm a little unclear from #2011, looks like this will require that we disable one/more options to get the current process per test behavior?

@novemberborn
Copy link
Member

So you're suggesting that ava would set process.env.AVA_TEST_FILE_FOR_DEDICATED_WORKER_PROCESS='test/feature1.js' that way an nyc --require hook could see that and perform desired manipulations of the configuration?

Precisely.

I'm a little unclear from #2011, looks like this will require that we disable one/more options to get the current process per test behavior?

We'll see where we settle on as far as the default behavior is concerned, but I can see how certain modes may not work with your use case. But then again they also won't work if you modify globals, so it'll be really project specific.

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.

3 participants