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

Simplify require_test.js, making it pass on Windows #337

Merged
merged 1 commit into from
Sep 30, 2017
Merged

Simplify require_test.js, making it pass on Windows #337

merged 1 commit into from
Sep 30, 2017

Conversation

edmorley
Copy link
Member

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.

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.
Copy link
Member

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perfect, and I like having less dependencies here. Thanks!

@eliperelman eliperelman merged commit 8c90e2f into neutrinojs:master Sep 30, 2017
@edmorley edmorley deleted the fix-require-test branch September 30, 2017 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

require_test.js failures on Windows during test-module directory cleanup
2 participants