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

tests: Possible fix the permission error when the tests open the cookie file #14788

Merged
merged 1 commit into from Dec 7, 2018

Conversation

Projects
None yet
4 participants
@ken2812221
Copy link
Member

commented Nov 22, 2018

This PR would wait until the .cookie file is readable
Possible fix no. 5 PermissionError in #14446

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Nov 22, 2018

utACK 0cb4518d355738b2530a66cded2fc4b673ba1c2a

@MarcoFalke MarcoFalke added the Tests label Nov 22, 2018

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 23, 2018

There's so much polling and waiting in the tests; from a determinism point of view I'd prefer to do this waiting once after starting a node, this is the only time where one'd expect the cookie to possibly not be there yet (maybe make it part of the current poll-after-run loop).

(If the same happens and it's trying to find the cookie after shutdown this is a problem with the test and it shouldn't simply wait 10 seconds. Or if the cookie goes missing while a node is supposed to be running this is a problem with the software that should be detected immediately.)

@promag

This comment has been minimized.

Copy link
Member

commented Nov 23, 2018

@ken2812221 ken2812221 force-pushed the ken2812221:fix-win-test-perm-deny branch to d6b3790 Nov 23, 2018

@ken2812221

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2018

@laanwj Now it would also raise ValueError if the cookie file is not readable. It can be caught in wait_for_rpc_connection()

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Dec 6, 2018

utACK d6b3790

@MarcoFalke MarcoFalke merged commit d6b3790 into bitcoin:master Dec 7, 2018

2 checks passed

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

MarcoFalke added a commit that referenced this pull request Dec 7, 2018

Merge #14788: tests: Possible fix the permission error when the tests…
… open the cookie file

d6b3790 tests: check readability of cookie file (Chun Kuan Lee)

Pull request description:

  This PR would wait until the `.cookie` file is readable
  Possible fix no. 5 `PermissionError` in #14446

Tree-SHA512: e7055c7ca26a6eadbbe19e4eef08ffee61cd17de79b30af2f0d090f0ad81ca24815e3c7e034e5e30d47c580bb0b221b3955e9ff2fcec2274fbf7b9232ab0cdc7

@ken2812221 ken2812221 deleted the ken2812221:fix-win-test-perm-deny branch Dec 8, 2018

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.