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

Problem with tests/bin/keys/reset.test.ts and tests/bin/keys/renew.test.ts stopping the PolykeyAgent #347

Closed
emmacasolin opened this issue Feb 22, 2022 · 5 comments · Fixed by #391
Assignees
Labels
bug Something isn't working r&d:polykey:supporting activity Supporting core activity

Comments

@emmacasolin
Copy link
Contributor

Describe the bug

When tests/bin/keys/reset.test.ts and tests/bin/keys/renew.test.ts use the shared global agent to run their tests (as opposed to their own separate agents) and are run at the same time, after the first test has finished running it will sometimes shut down the global agent even though there are still other processes using it. It will do this only if the other test is reading from the global data directory at the same time as when the first test is attempting to stop:

INFO:KeyManager:Reading /tmp/polykey-test-global-6W6c7T/agent/state/keys/db.key
INFO:KeyManager:Reading /tmp/polykey-test-global-6W6c7T/agent/state/keys/vault.key
INFO:renew test:Stopping PolykeyAgent

To Reproduce

  1. Checkout dc53883
  2. Run npm test -- ./tests/bin/keys/reset.test.ts ./tests/bin/keys/renew.test.ts (note that putting renew before reset seems to not reproduce the same issue)

Expected behavior

The Stopping PolykeyAgent message can only come from the PolykeyAgent.stop method, however, this is not being called from inside globalAgentClose, as would be expected. We need to trace where this is being called from as it should only be called from globalAgentClose when using the global agent, which should only happen after all tests have finished running.

Additional context

@CMCDragonkai
Copy link
Member

This was worked around first by making these tests that use renew and reset use their own agent and not the global agent.

@CMCDragonkai
Copy link
Member

Might be related to #386.

@CMCDragonkai
Copy link
Member

@tegefaulkes can you incorporate this since you've been removing the global agent. If we are removing the global agent, perhaps this issue can be resolved?

@CMCDragonkai
Copy link
Member

Connect this issue to #404 if it is relevant.

@CMCDragonkai
Copy link
Member

For these 2 tests, it was already swapped to using per-module agent.

Now with #404, these 2 tests are swapping from per-test-module agent to using global keypair.

Once it is using the global keypair fixtures, the per-test-module agent is now becoming a per-test-function agent. This means there no longer any state sharing of the agent across test functions.

So this issue will be closed once it is merged, but we should be wary if there's something broken in the key reset and key renew in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working r&d:polykey:supporting activity Supporting core activity
Development

Successfully merging a pull request may close this issue.

3 participants