Skip to content

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Sep 3, 2020

Needed to (manually and locally) test our overly complex error handling framework, see for instance PR #354

Signed-off-by: Marc Herbert marc.herbert@intel.com

@marc-hb marc-hb marked this pull request as ready for review September 3, 2020 08:05
@marc-hb marc-hb requested a review from a team as a code owner September 3, 2020 08:05
Copy link
Contributor

@xiulipan xiulipan left a comment

Choose a reason for hiding this comment

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

@marc-hb why this fake_kern_error function is needed? And where we will use this function?
Please also add some explanation in the commit.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Sep 7, 2020

@xiulipan what part of the commit message needs more detail?

@xiulipan
Copy link
Contributor

xiulipan commented Sep 8, 2020

@marc-hb For the commit case-lib/lib.sh: add new fake_kern_error(). Can you explain why we need to add such a function?
I do not think some discussion on GitHub can answer the question when people looking for the source code. (Please also add some inline comments if this function is for test only)

PS: will this function take effect also with journalctl?

Sort of fixes commit 8b02299 ("fix pulseaudio re-start command
line"). The whole pulseaudio situation is a mess anyway, see PR thesofproject#300 and
others.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb force-pushed the fake-kern-error branch 2 times, most recently from 65966a7 to 875acd9 Compare September 8, 2020 04:58
@marc-hb
Copy link
Collaborator Author

marc-hb commented Sep 8, 2020

Thanks @xiulipan for the review, I added more details.

PS: will this function take effect also with journalctl?

Yes. Unlike kern.log, journalctl saves everything from dmesg.

This function adds a fake error to dmesg.

It is surprisingly easy to write a test that always succeeds, especially
in shell script and it has already happened a number of
times. Temporarily add this function to a test under development to make
sure it can actually report failures. Using this function is even more
critical for testing changes to the test framework and especially error
handling code.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Sep 8, 2020

Besides the regression mentioned in #354, here's another example of bug found thanks to this function: #366

@xiulipan
Copy link
Contributor

xiulipan commented Sep 9, 2020

@marc-hb I got the idea now. Can you also add some DEBUG prefix in commit and inline comments to show this function should only be used for debug.

PS: I think it would be better if you can have some unit test for our lib with this debug or test function.

@xiulipan xiulipan merged commit 926d8a1 into thesofproject:master Sep 10, 2020
@marc-hb marc-hb deleted the fake-kern-error branch September 14, 2020 04:24
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.

2 participants