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

minor fixes in e2e tests ext_cgroups, agent_update and agent_publish #3199

Merged
merged 4 commits into from
Sep 3, 2024

Conversation

nagworld9
Copy link
Contributor

@nagworld9 nagworld9 commented Aug 28, 2024

Description

ext_cgrups: run_remote_test swallowing the assertion error, so test failed to validate if assertion failed due to cgroups disabled. So removed calling run_remote_test

agent_update: when rsm_request sent, whole pipeline can take some time to update the goal state with the requested version, so increasing the timeout on that check

agent_publish: Tests are running before agent is ready. Added check for wait for agent to complete provisioning

Issue #


PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • If applicable, the PR references the bug/issue that it fixes in the description.
  • New Unit tests were added for the changes made

Quality of Code and Contribution Guidelines

@@ -68,6 +71,22 @@ def get_ignore_errors_before_timestamp(self) -> datetime:
timestamp = self._ssh_client.run_command("agent_publish-get_agent_log_record_timestamp.py")
return datetime.strptime(timestamp.strip(), u'%Y-%m-%d %H:%M:%S.%f')

def _wait_for_agent_to_complete_provisioning(self):
Copy link
Member

Choose a reason for hiding this comment

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

I'd consider moving this somewhere in testss/lib, since it looks like a useful function that other tests could use

log.info("Waiting for agent to complete provisioning, will check again after a short delay")

else:
fail("Timeout while waiting for the Agent to complete provisioning")
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be "raise Exception" rather than "fail".

Both of them raise an exception, but fail/assert/etc should be used when the verifications the test is doing completed, but were not True. Other exceptions would be used when the test was trying to setup something, or waiting for something before doing its verifications like in this case, etc, and some error prevented the test to perform its verifications.

@nagworld9 nagworld9 merged commit 96975fa into Azure:develop Sep 3, 2024
11 checks passed
@nagworld9 nagworld9 deleted the ext-cgroups-check branch September 3, 2024 17:05
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.

3 participants