Skip to content

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented May 2, 2022

See commit messages.

marc-hb added 5 commits May 2, 2022 16:04
To help with thesofproject#312, commit 2e24024 ("test-case: add set -e to 7
function test") blindly added "set -e" without reviewing the code of any
test and without making sure it's actually compatible with set -e. In
this test this meant missing the rtcwake exit code.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Replace with a simpler and safer grep -q.

No functional change.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
… sudo

Remove the dependency on hijack.sh#sudo() and simplify the multiple
layers of quoting. Fixes the following error when running with plain
sudo:

bash: echo deep > /sys/power/mem_sleep: No such file or directory

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
We should never pass an invalid argument.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
shellcheck source and BASH_SOURCE quoting

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb marked this pull request as ready for review May 3, 2022 01:11
@marc-hb marc-hb requested a review from a team as a code owner May 3, 2022 01:11
@marc-hb
Copy link
Collaborator Author

marc-hb commented May 3, 2022

None of the two errors in https://sof-ci.01.org/softestpr/PR899/build15/devicetest/ are new.

Copy link

@paulstelian97 paulstelian97 left a comment

Choose a reason for hiding this comment

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

I like this for the most part, the fact that the new code is more readable (on one of the commits I didn't understand the old code but I kinda do the new one, so I leave it to you to confirm that functionality doesn't really change) is a good thing.

Just one comment that I want to get clarified.

grep -q "$type" /sys/power/mem_sleep || {
grep -H '^' /sys/power/mem_sleep
skip_test "Unsupported sleep type argument: $type"
die "Unsupported sleep type argument: $type"

Choose a reason for hiding this comment

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

Two changes on the same thing? Feels odd but okay...

Copy link
Collaborator Author

@marc-hb marc-hb May 3, 2022

Choose a reason for hiding this comment

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

The first commit here is a no-change cleanup that will stay. The second one switches the SKIP to a FAIL, it could be reverted if it is found to break "backwards compatibility" somewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

What does the following line do here?
grep -H '^' /sys/power/mem_sleep

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's just a very fancy cat; try it on any file and you will see every line is prefixed by the file name.

grep -q "$type" /sys/power/mem_sleep || {
grep -H '^' /sys/power/mem_sleep
skip_test "Unsupported sleep type argument: $type"
die "Unsupported sleep type argument: $type"
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the following line do here?
grep -H '^' /sys/power/mem_sleep

@marc-hb
Copy link
Collaborator Author

marc-hb commented May 4, 2022

Thanks @greg-intel . Very small code changes with almost no functional change, very easily reverted if needed -> merging.

@marc-hb marc-hb merged commit 287a6b9 into thesofproject:main May 4, 2022
@marc-hb marc-hb deleted the suspend-fixes branch May 4, 2022 17:52
@marc-hb
Copy link
Collaborator Author

marc-hb commented Jul 26, 2022

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