test/system: make some tests faster part 2 #22826
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci
bot
added
release-note-none
approved
Luap99
commented
May 28, 2024
There was a problem hiding this comment.
# #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
# #| FAIL: Sequential output from logs
# #| expected: =~ e1e85a9e2107 a
# #| > 640d8927bc2f b
# #| > 640d8927bc2f c
# #| > e1e85a9e2107 d
# #| actual: 640d8927bc2f b
# #| > 640d8927bc2f c
# #| > e1e85a9e2107 a
# #| > e1e85a9e2107 d
Oh well so much to my long testing for this one, DO NOT MERGE!
Luap99
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@edsantiago How much do you are about the fact that "podman logs - multi $driver" wants this output? There is no way to guarantee ordering on reading end in podman logs. The only way to do so is with the ugly sleep's that were used before. So I wonder do I add some sleep's back or are you fine with we changing the tests to accept any order as long as the lines for the individual container are in the right order, i.e.
|
|
That's not something I care about. In theory it would be nice but if you think it's okay to waive that requirement, and other reviewers agree, I'm fine with loosening |
|
These changes LGTM |
There is no reason for this check to wait 4 seconds for the container to run, instead make sure to have a running process and then stop it directly with -t0 not have any delay. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The test used sleep to synchronize log output between both containers which is slow. There is actually no way to guarantee the ordering on the reading side so just remove the sleep's and check the the lines within the same container are in the right order. Trying to preserve the orignal ordering is just not possible if we speed up the test as it would flake to often. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Instead of iterating over all tmp dirs and creating test containers for each one we can just pass all files to one touch call. With that we have to create much less containers while still checking the same thing. This speeds up the test by about 4 seconds. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Do not wait 5 seconds, just stop the container directly. This speeds up the test by more than 4 seconds. One could make the case here that we want to check podman wait but there are so many other podman wait tests that it should not matter. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Use only one retry and a short stop timeout to speed them up. I am not sure if this will cause flakes, I have not seen any after trying for some time so I think this works just as well. And is about 2-3 seconds faster for both tests. If it does start to flake we can revert this commit again or write the test differently. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The current logic used podman logs I don't understand way, all we care about is the container output and we can just read the same with a attached podman run, of course we have to move it into the background but it did the some with logs. This also allows us to remove the extra log-driver checks and because podman logs seems to be much slower than the extra run we safe over 10s with this change. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This test is by far the slowest one taking over minute, the reason is that it is checking every single podman command for shell completions. The test is useful but it does not need to check the "..." argument 3 times. Test a second time to make sure not only the first arg is completed. This change makes it about 15 seconds faster. Long term we should get this test out of the main system tests together with other cli only tests as they do not need to run on each OS, etc... Signed-off-by: Paul Holzinger <pholzing@redhat.com>
use a command that stops on SIGTERM not sleep, that way the tests can continue to use podman kube down without waiting for the full stop timeout every time. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The command does not react on sigterm, so kube down needs to wait 10s. To fix it first use a command that does but also write the yaml directly instead of doing the podman create && kube generate dance. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Luap99
force-pushed
the
fast-system-test-2
branch
from
1b76203
to
ad661b5
Compare
Luap99
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So many tests use sleep or run into the default 10s stop timeout, this is just part 2 of many
Part 1 #22821
Does this PR introduce a user-facing change?