Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

require_test.js failures on Windows during test-module directory cleanup #336

Closed
edmorley opened this issue Sep 29, 2017 · 1 comment · Fixed by #337
Closed

require_test.js failures on Windows during test-module directory cleanup #336

edmorley opened this issue Sep 29, 2017 · 1 comment · Fixed by #337
Assignees

Comments

@edmorley
Copy link
Member

On master, if I run yarn && yarn bootstrap && yarn test on Windows, I get:

  neutrino » test » require_test » after

  Rejected promise returned by test. Reason:

  Error {
    code: 'EBUSY',
    errno: -4082,
    path: 'C:\\Users\\Ed\\src\\_todo\\neutrino-dev\\packages\\neutrino\\test\\test-module',
    syscall: 'rmdir',
    message: 'EBUSY: resource busy or locked, rmdir \'C:\\Users\\Ed\\src\\_todo\\neutrino-dev\\packages\\neutrino\\test\\test-module\'',
  }

Running yarn test again after that, shows not only that error, but also another one:

  Uncaught Exception
  SyntaxError: C:/Users/Ed/src/_todo/neutrino-dev/packages/neutrino/test/test-module/errorMiddleware.js: Unexpected token (1:1)
...
  × No tests found in packages\neutrino\test\test-module\middleware.js, make sure to import "ava" at the top of your test file

The first error is resolved if I move the process.chdir(cwd); call to before remove(rootPath) - presumably Linux/OS X is fine with removing a directory when it's the current directory but not Windows.

The second error is due to the test-modules directory being left behind in the case of the test cleanup throwing, and then ava finding those files during test discovery. Whilst this will only occur in the case of a previously failed run, it would be great to prevent it, by doing one of:
a) Moving the test-modules directory to somewhere in os.tmpdir() instead
b) Renaming test-modules to something that is excluded from test discovery (eg helpers, fixtures - see here)

I have a branch locally that solves both of these - however it then occurred to me why is this test creating the test-module directory on every run, rather than it being committed to the repo under the existing packages/neutrino/test/fixtures directory? Doing this would avoid the above, speed up the test, and mean we can drop the fs-extra and pify devDependencies.

@eliperelman - thoughts on just turning this into a fixture committed to the repo?

@edmorley
Copy link
Member Author

@eliperelman - thoughts on just turning this into a fixture committed to the repo?

I thought it might just be easier to open a PR for this approach, since it'll be easier to discuss: #337.

eliperelman pushed a commit that referenced this issue Sep 30, 2017
Previously the test would consistently fail on Windows due to it
attempting to remove the `test-modules` directory before the `chdir()`
call to reset the working directory. Subsequent runs would additionally
hit ava test discovery errors due to it seeing the leftover
`test-modules` directory as a test rather than a fixture.

Whilst these issues could be fixed by inverting the cleanup order, it
makes more sense to commit the `test-module` files to the repo rather
than generate them on the fly. They have been moved under the `fixtures`
directory to avoid the above ava errors.

The test cleanup `chdir(cwd)` step has also been removed, since ava runs
each test file in a new process, meaning no tests will run after it
anyway.

Fixes #336.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant