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

[CTK-4365] [CTK-4612] [CTK-4612] Update tests to look at all possible payloads to look for valid processes instead of just the first 2 #33642

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wiyu
Copy link
Contributor

@wiyu wiyu commented Jan 31, 2025

What does this PR do?

Moves the payload assertions into the assertEventually that way we can capture up to 2 minutes worth of payloads instead of just the first 2. This is to reduce flakiness in case the cpu-stress processes do not show cpu load on the first 2 initial payloads due to startup.

This updates the ECSEC2 test and the Docker Process Check. If this doesn't flake as much, then we can convert all tests to follow this method of assertion.

Motivation

Reduce test flakes

Describe how you validated your changes

Possible Drawbacks / Trade-offs

Additional Notes

wiyu added 3 commits January 31, 2025 13:09
…assertEventually. Also introduce more granular error logging.
…d processes instead of just the first 2. This is to reduce flakiness of the tests.
…d processes instead of just the first 2. This is to reduce flakiness of the tests.
@wiyu wiyu added changelog/no-changelog qa/no-code-change No code change in Agent code requiring validation labels Jan 31, 2025
@wiyu wiyu added this to the 7.64.0 milestone Jan 31, 2025
@wiyu wiyu requested a review from a team as a code owner January 31, 2025 21:59
@github-actions github-actions bot added the short review PR is simple enough to be reviewed quickly label Jan 31, 2025
@@ -136,6 +185,17 @@ func findProcess(
return found, populated
}

// filterProcesses returns processes which match the given process name
Copy link
Contributor Author

@wiyu wiyu Jan 31, 2025

Choose a reason for hiding this comment

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

I broke this out from findProcess() as to allow for better logging of when we don't find cpu/memory data from the processes. When we are missing that data we can just log the processes we found instead of all payloads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog qa/no-code-change No code change in Agent code requiring validation short review PR is simple enough to be reviewed quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant