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

Unit test cleanup #75

Merged
merged 20 commits into from
Oct 25, 2021
Merged

Unit test cleanup #75

merged 20 commits into from
Oct 25, 2021

Conversation

TwitchBronBron
Copy link
Member

Fix many issues in the unit tests. Remove reliance on the test project in favor of creating files in each unit test instead.

I had to do some package upgrading too to get things working. This replaces #74 because this PR fixes the unit tests that one couldn't resolve.

@TwitchBronBron
Copy link
Member Author

Closing in favor of #75

@TwitchBronBron
Copy link
Member Author

Oops, closed the wrong one!

@TwitchBronBron
Copy link
Member Author

Something isn't right. The code running in those "push" builds don't seem to be running the correct code.

@TwitchBronBron TwitchBronBron marked this pull request as draft October 23, 2021 02:50
@TwitchBronBron TwitchBronBron marked this pull request as ready for review October 23, 2021 02:50
@TwitchBronBron
Copy link
Member Author

Finally figured it out. The whole problem with these tests were the createReadStream and createWriteStream functions. When an exception occurred, they weren't getting disposed properly in all situations.

"nyc": "^15.1.0",
"rimraf": "^2.6.2",
"sinon": "^11.1.2",
"source-map-support": "^0.5.13",
"testdouble": "^3.5.2",
"ts-mocha": "^6.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this because mocha has built-in support for loading typescript (as seen in the mocha config below)

"mocha": "^9.1.3",
"node-run-cmd": "^1.0.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

remove this in favor of using child_process directly

@TwitchBronBron TwitchBronBron merged commit 59f5532 into master Oct 25, 2021
@TwitchBronBron TwitchBronBron deleted the unit-test-cleanup branch October 25, 2021 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants