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

Cleanup test files on Windows after executing test binary #1910

Merged
merged 3 commits into from Apr 19, 2019

Conversation

@wezrule
Copy link
Collaborator

commented Apr 18, 2019

The test results on Windows include a lot of noise after most tests:
Could not remove temporary directory: The process cannot access the file because it is being used by another process

These temporary folders can build up after time (many GB in my case). In particular the files which are still held are the logging file and the store/wallet store database files. The logger is set up as a singleton for the duration of the program, so is not easily closed. It's also a nightmare trying to stop the store/wallet store in node::stop as other stuff can still execute in the background which can talk to the database, pending alarm operations in particular. So we need to wait until the node destructor has finished. I could probably add a "stopped" flag in a load of places, but I feel like it could introduce problems and this is only for tests do didn't want to add any regression risks.

An option would be to change all test to use test fixtures and make use of the TearDown () function to contain the cleaning logic. But it involves changing every test, e.g:

TEST (wallet, update_work_action)
becomes:
TEST_F (TestFixtureWithTearDownImplemented, wallet.update_work_action)

Not very ideal, but it would mean all platforms would clean up correctly after each test is run. I will leave this option open for debate.

For now, I've implemented conditional compilation so that on Windows the cleaning up only happens at the end of the test executable. This does mean that early exiting in any test will not clean up any proceeding but it seems better than nothing (and also removes the noisey output). The other platforms remain unchanged.

@wezrule wezrule added this to the V19.0 milestone Apr 18, 2019

@wezrule wezrule requested review from argakiig and cryptocode Apr 18, 2019

@wezrule wezrule self-assigned this Apr 18, 2019

@wezrule wezrule changed the title Cleanup test files on Wtraindows after executing test script Cleanup test files on Windows after executing test script Apr 18, 2019

@wezrule wezrule changed the title Cleanup test files on Windows after executing test script Cleanup test files on Windows after executing test binary Apr 18, 2019

@wezrule wezrule removed this from the V19.0 milestone Apr 18, 2019

@wezrule wezrule added this to During RC in V19 Apr 18, 2019

@wezrule wezrule added this to the V19.0 milestone Apr 18, 2019

@cryptocode
Copy link
Collaborator

left a comment

LGTM

@wezrule wezrule merged commit ebcd2dd into nanocurrency:master Apr 19, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wezrule wezrule deleted the wezrule:cleanup_test_files_windows branch Apr 19, 2019

@zhyatt zhyatt moved this from During RC to RC2 (2019-05-06) in V19 May 6, 2019

@zhyatt zhyatt moved this from RC2 (2019-05-06) to CP3/RC 1 (2019-04-26) in V19 May 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.