Skip to content

Fix Inter Suite Cross Contamination #673

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

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

wiktormowinski
Copy link
Contributor

Some tests behave differently when launching them inside whole suite vs individually. This fact suggest that after one suite is complete (by returning either Pass Fail or Skip) the environment doesn't return to it's original state. The most common (but not the only one) manifestation of this issue is telnet related error: "EOFError: telnet connection closed"

This MR aims to find these problematic spots and in effect, bring the regression and individually run tests as close in result as possible.

Signed-off-by: Wiktor Mowinski <wiktor.mowinski@3mdeb.com>
Signed-off-by: Wiktor Mowinski <wiktor.mowinski@3mdeb.com>
@wiktormowinski
Copy link
Contributor Author

wiktormowinski commented Jan 16, 2025

The change in APU suite teardown we did a while ago was a good addition and served its purpose well. However, it seems like it is causing a new problem to tests following.
image
Figure. 1 The affected area: suites between APU and CBK (being the first suite where it starts to PASS)

After trying a few fixes i've finally found the culprit. The Skip not only skips the obsolete reflash it also gets rid of Log Out And Close Connection. The absence of this keyword is connected to the weird behavior in the next few suites. So after forcibly adding the missing kwd before Skip If the results are looking good:
image
Figure. 2 The affected area: suites between APU and CBK
NOTE- the few audio and cpu tests failed because of different issues (for example missing config contents)

The experiment was a success. The only problem now is how do we handle this issue @macpijan ? As far as I know, we can't put condition in teardown directly, that's why these commits: 8474e5d dcb979c created a kwd just for this occasion (the name is wip)

Here are the logs:
logs-fixes.zip

@philipanda
Copy link
Contributor

As far as I know, we can't put condition in teardown directly

There is the keyword "Run Keyword If" which allows to run another keyword if a condition is true. It should be possible to run it from "Run Keyword" in Suite Setup / Suite Teardown

@SebastianCzapla
Copy link
Contributor

Probably the best solution here is to change contributing guidelines, and enforce suite init/teardown as local keywords:
image
image

It has multiple benefits, clear skip conditions, we can install required packages as part of the initialization, we can set dut into any state we need, and we can clearly unmake the changes, and return to the default state DUT

@philipanda
Copy link
Contributor

I think that creating local keywords for setup and teardown won't allow to achieve more than using Run Keywords for the procedures, like it is now. What will it help with is making complex setup & teardown procedures easier to create and read.

I am not sure about enforcing defining those procedures as keywords.

  • Complex setup/teardown is not needed in most test suites.
  • All keywords in robot framework are of global scope. You can add the tag robot:private, but all that achieves is throwing a warning if the keyword is used. Even the libdoc does not care about the tag (as of version 5.0 we are using, the latest version won't document them Upgrade to RF7 #606)

@macpijan macpijan closed this Jul 3, 2025
@macpijan macpijan deleted the regression-telnet-bugfix branch July 3, 2025 13:46
@philipanda philipanda restored the regression-telnet-bugfix branch July 3, 2025 14:04
@philipanda philipanda reopened this Jul 3, 2025
@wiktormowinski
Copy link
Contributor Author

related: #930

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.

4 participants