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

Agent spawning tests require refactoring. #238

Closed
4 tasks
tegefaulkes opened this issue Sep 3, 2021 · 2 comments · Fixed by #283
Closed
4 tasks

Agent spawning tests require refactoring. #238

tegefaulkes opened this issue Sep 3, 2021 · 2 comments · Fixed by #283
Assignees
Labels
development Standard development epic Big issue with multiple subissues

Comments

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Sep 3, 2021

Specification

The following tests need to reviewed and refactored.

  • tests/bin/agent.test.ts > Starting the agent in the foreground > should start the agent and clean up the lockfile when a kill signal is received
  • tests/bin/agent.test.ts > Starting the agent in the background > should start the agent and clean up the lockfile when a kill signal is received
  • tests/agent/utils.test.ts > spawnBackgroundAgent > Should spawn an agent in the background.

They all spawn an agent and wait a set amount of time before checking if it is online. During CI/CD there is a good chance that the time it takes for the agents to start exceed the arbitrary delay waiting for them to start. this is causing the tests to fail. The reason for the delay is that currently there is no good feedback for when the agent has fully started up. we need to explore a better more robust way to do theses tests, work out a better way to tell if the agent has started.

Additional context

Currently to get things working I've created a simple polling function to check periodically for a true condition.

async function poll(
  timeout: number,
  condition: () => Promise<boolean>,
  delay: number = 1000,
) {
  let timeProgress = 0;
  while (timeProgress < timeout) {
    if (await condition()) break;
    await sleep(delay)
    timeProgress += delay;
  }
  expect(await condition()).toBeTruthy();
}

This too could use some improvement. It will run a function that you provide every delay. if the function returns true or the polling look exceeds the timeout then the expect() is run. It's useful to provide a large delay for waiting for a condition to be true without waiting the full delay. This only solves part of the problem though.

Tasks

  1. Determine a more robust way to do these tests.
  2. Refactor the tests.
  3. Check that tests pass.
  4. Check that CI/CD passes.
@CMCDragonkai
Copy link
Member

@tegefaulkes is this resolved by our work backporting Status into the https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/213

@tegefaulkes
Copy link
Contributor Author

I'll have this closed by the merge of #283 then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development epic Big issue with multiple subissues
Development

Successfully merging a pull request may close this issue.

4 participants