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

config: rt-tests: fix MLOCKALL getting set to True #2668

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

musamaanjum
Copy link
Contributor

When MLOCKALL is set to 'true', the lava job has MLOCKALL becomes true [1] which in turn translates to "True" while job execution [2]. It causes error as the test expects true instead of True.

./pi-stress.sh -D 60s -m True -r false -w hackbench
./pi-stress.sh: 37: True: not found

Fix it by using double quotes instead.

[1] https://lava.collabora.dev/scheduler/job/15562666/definition#defline43
[2] https://lava.collabora.dev/scheduler/job/15562666#L220

@musamaanjum
Copy link
Contributor Author

Let's confirm from the execution results if this works.

@musamaanjum
Copy link
Contributor Author

musamaanjum commented Sep 20, 2024

This fix didn't work: https://lava.collabora.dev/scheduler/job/15747114

@pawiecz please can you help solve this issue? In job (https://lava.collabora.dev/scheduler/job/15747239), I've manually added double quotes around true and job worked fine.

@musamaanjum
Copy link
Contributor Author

This fix didn't work: https://lava.collabora.dev/scheduler/job/15747114

@pawiecz please can you help solve this issue? In job (https://lava.collabora.dev/scheduler/job/15747239), I've manually added double quotes around true and job worked fine.

@pawiecz Please have a look.

config/runtime/tests/rt-tests.jinja2 Outdated Show resolved Hide resolved
@pawiecz
Copy link
Contributor

pawiecz commented Sep 25, 2024

Also: timeout for this test suite has to be extended to provide some buffer for initialization - it shouldn't be equal to the requested test duration.

When MLOCKALL is set to 'true', the lava job has MLOCKALL becomes true
[1] which in turn translates to "True" while job execution [2]. It
causes error as the test expects true instead of True.

  ./pi-stress.sh -D 60s -m True -r false -w hackbench
  ./pi-stress.sh: 37: True: not found

Fix it by using double quotes inside single quotes.
- First set of quotations allows Jinja template engine to treat it as a
  string
- Second set is for LAVA to also treat is as string and prevent YAML
  parser from making it a bool

[3] is successful job after this change.

[1] https://lava.collabora.dev/scheduler/job/15562666/definition#defline43
[2] https://lava.collabora.dev/scheduler/job/15562666#L220
[3] https://lava.collabora.dev/scheduler/job/15790058

Relates kernelci/kernelci-project#439
Co-authored-by: Paweł Wieczorek <[email protected]>
Signed-off-by: Muhammad Usama Anjum <[email protected]>
@musamaanjum
Copy link
Contributor Author

Also: timeout for this test suite has to be extended to provide some buffer for initialization - it shouldn't be equal to the requested test duration.

@pawiecz we had probably discussed this in some meetings that it is okay to have the same timeouts. Or I may be mistaken. Currently, the total test duration is set to 10 minutes and the test's execution is also set to 10 minutes. Should we reduce the test duration by 30 or 60 seconds?

@musamaanjum musamaanjum requested a review from pawiecz September 25, 2024 13:27
@musamaanjum
Copy link
Contributor Author

Result: https://lava.collabora.dev/scheduler/job/15790240

@nuclearcat let's merge this PR.

@pawiecz
Copy link
Contributor

pawiecz commented Sep 26, 2024

Also: timeout for this test suite has to be extended to provide some buffer for initialization - it shouldn't be equal to the requested test duration.

@pawiecz we had probably discussed this in some meetings that it is okay to have the same timeouts. Or I may be mistaken. Currently, the total test duration is set to 10 minutes and the test's execution is also set to 10 minutes.

Let me give an example:

Due to these slight delays it might not be able to finish successfully. Providing a slight margin (e.g. 11 minutes instead of 10 for a test expected to go for 10 minutes) could reduce flakiness

Should we reduce the test duration by 30 or 60 seconds?

I'm not sure how long this test case should run to provide meaningful results 😢

Copy link
Contributor

@pawiecz pawiecz left a comment

Choose a reason for hiding this comment

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

LGTM

(Setting right timeouts is a separate item)

@nuclearcat nuclearcat added this pull request to the merge queue Sep 28, 2024
Merged via the queue into kernelci:main with commit 3b8eb56 Sep 28, 2024
4 checks passed
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