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

Enable suspend and hibernation test #743

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Enable suspend and hibernation test #743

merged 1 commit into from
Sep 30, 2024

Conversation

crazoes
Copy link

@crazoes crazoes commented Aug 2, 2024

Enable suspend and hibernation test. This patch depends on the #743

collections: cpufreq
job_timeout: 10
env: 'KSELFTEST_MAIN_SH_ARGS="-t suspend_rtc"'
kcidb_test_suite: kselftest.cpufreq
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we need to define a different kcidb_test_suite here, as this is the same cpufreq kselftest but run with different parameters. @JenySadadia is there an existing convention for this? Users need a way to differentiate between the same test executed with different parameters once the results land in KCIDB.
(This goes back to the discussion raised here)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think KCIDB has such conventions. Maybe we can use kselftest.cpufreq.suspend and kselftest.cpufreq.hibernate.
CC @spbnick

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me, thanks @JenySadadia ! About the parameters passed to this test, is there any field we can add or re-use to encode this information? besides being useful for filtering results, it's also good information for manually reproducing the issues.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One way is to store that information in node data field and later on assign that to misc field for KCIDB submission.

@laura-nao
Copy link
Contributor

Hi @crazoes - were you planning to add only the test definition at this stage, or also to run the test on some devices? If it’s the latter, you’ll need to include the test in the scheduler: section (either in pipeline.yaml or scheduler-chromeos.yaml), similar to what was done in #588

@crazoes
Copy link
Author

crazoes commented Aug 2, 2024

@laura-nao I just wanted to first see if this approach is correct so thought of only adding the test definition. But I can make changes in my v2 as per the reviews

@a-wai a-wai added the staging-skip Don't test automatically on staging.kernelci.org label Aug 6, 2024
config/pipeline.yaml Outdated Show resolved Hide resolved
config/pipeline.yaml Outdated Show resolved Hide resolved
@pawiecz pawiecz removed the staging-skip Don't test automatically on staging.kernelci.org label Aug 21, 2024
@a-wai
Copy link
Contributor

a-wai commented Aug 23, 2024

There are still errors on the kci-core side, but this PR is working as expected: multiples instances of kselftest-cpufreq-{hibernate,suspend} in this node

@crazoes
Copy link
Author

crazoes commented Aug 27, 2024

@a-wai @pawiecz I updated this PR to add a rule for running this test only on kernelci/staging-next. Hopefully this will fix the failing test.

@a-wai
Copy link
Contributor

a-wai commented Aug 27, 2024

Looks good, can you please invert the entries in pipeline.yaml? -suspend should be after -hibernate based on alphabetical order (kci config validate should tell you so, but we don't have that check in CI yet).

@crazoes
Copy link
Author

crazoes commented Sep 3, 2024

If everything looks good then can we merged this now @nuclearcat @a-wai ?

<<: *kselftest-params
collections: cpufreq
env: 'KSELFTEST_MAIN_SH_ARGS="-t hibernate_rtc"'
rules:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we want to run this test on all the trees and not just kernelci. I may not have enough context for this. So what do you think?

@nuclearcat
Copy link
Member

Did we got results of tests on dashboard or in viewer?

@crazoes
Copy link
Author

crazoes commented Sep 23, 2024

@nuclearcat yes we did get them but lots of tests are failing. https://lava.collabora.dev/scheduler/alljobs?length=25&search=suspend#table

It's not really the problem related to the tests or environment but mostly related to the kernel I think.

@nuclearcat
Copy link
Member

Thanks! I think soon we will start looking into such problems more.

@nuclearcat nuclearcat added this pull request to the merge queue Sep 30, 2024
Merged via the queue into main with commit 80ec2c4 Sep 30, 2024
3 checks passed
@nuclearcat nuclearcat deleted the suspend-resume-test branch September 30, 2024 09:29
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.

7 participants