test: enable linter on test.js#275
Conversation
|
The CI failure seem to be related to mysticatea/eslint-plugin-node#277, which was merged but hasn't been published yet. |
|
The approach seems OK to me if you can get the linter to succeed on test.js. Maybe include a comment in fixturesUrl.js explaining that it's there because current ESLint can't parse |
Removing |
| // Test that correctly-formatted markdown is ok. | ||
| (async () => { | ||
| const file = await read(new URL("./fixtures/ok.md", import.meta.url)); | ||
| const file = await read(new URL("./ok.md", fixturesUrl)); |
There was a problem hiding this comment.
Why is the fixtures being dropped from the path here? Is it related to the move of that import?
There was a problem hiding this comment.
We're importing the fixturesUrl, which is a file: URL leading to the fixtures/ folder, so yes it is related to the move of that import :) (import.meta.url is an absolute URL leading to the module where it's executed, not relative to the cwd or the module importing it).
There was a problem hiding this comment.
OK, maybe that is just more confusing for me 😄
Would pulling it back up to the test root, and just ignoreing the file path in eslintignore (instead of the directory) make sense? Don't feel strongly about it, but it wouldn't be obvious on why the paths worked like this if I was going to write a new test
There was a problem hiding this comment.
Would it help if we renamed fixturesUrl into something else? Something that makes it obvious that the relative URL is based on the fixtures directory.
aduh95
force-pushed
the
lint-test-module
branch
from
9b0e0ab to
fbb0209
Compare
3 participants
No description provided.