-
Notifications
You must be signed in to change notification settings - Fork 744
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
ACL tests are taking a long time to run in master #12715
Comments
@arista-nwolfe, the time is increased because each test is repeating all the fixtures, i.e. acl tabl creation, setup, populate_vlan_arp_entries acl rule creation and then teardown of each of the above fixtures, hence increasing the total time for each test case.
|
@vperumal thanks for the details, do you happen to know why this change was made to run all fixtures on each test instead of once per paramertization? Or do you have a link to the PR which made that change? |
Assign to @cyw233 as discussed |
I noticed that the parameters of the tests changed between master and 202305. Master: 202305: I reverted this PR which appears to have added that extra After that I see the individual tests don't run the long running fixtures
|
@arista-nwolfe - Yes, It looks like that is the cause. The extra nfc407-5 is the dut parameterization. But the root cause as to why it causes this issue seems to be an issue in pytest - pytest-dev/pytest#12146. They think this may fix it - pytest-dev/pytest#11257 but it is still not merged or confirmed yet. |
Ah good find @vperumal, thanks for the reference |
Hey @arista-nwolfe @vperumal, sorry I was late to the party. It seems like the logs are fine on my end, for example, there are only 2 |
Hi @cyw233 - the issue is reproducible when you have multi-duts - For e.g. T2 profile. ACL script runs on the whole chassis, it doesn't repeat the test for each DUT. The tests are repeated for other parameters, direction, v4/v6 etc. The change introduced in #11614 causes the test to run on each DUT, which in turn causes individual fixtures to repeat. Any script similar to ACL will face the same problem. we will need to revert #11614 to fix this issue. |
Thank you @vperumal for the info! Let me talk to the author of the PR and find the best way to resolve this. Cheers. |
Hey @arista-nwolfe , yes that PR should fix this issue. Can you please confirm on your end as well? Thanks! |
@patrickmacarthur confirmed that this PR fixed this issue. Thanks! |
Issue Description
The sonic-mgmt ACL tests have increased in runtime duration significantly between the 202205 to master branch.
Results you see
Here are a few of the test runtime outputs on master:
Results you expected to see
Here are those same test runtime outputs on 202205:
Note the amount of time between the first two testlets is similar but then subsequent tests on 202205 would take 1s while on master they take ~5 minutes each.
Also note that
test_egress_unmatched_forwarded[ipv4-ingress-downlink->uplink-default-no_vlan]
isSKIPPED
but it still takes ~5 minutes on master so this suggests it maybe the test fixtures which are consuming so much time on master.Is it platform specific
generic
Relevant log output
No response
Output of
show version
No response
Attach files (if any)
No response
The text was updated successfully, but these errors were encountered: