Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

plugins/linux_log_parser: Add support for test name regex #1142

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

katieworton
Copy link
Member

Add support for a test name regex to extract parts of the log parser outputs to produce more meaningful test names.

Instead of producing test names with the "REGEX_NAME" + SHA, create test names with "REGEX_NAME"+"REGEX_EXTRACT_NAME"+SHA to make the test names easier to interpret.

Example test name before:
check-kernel-oops-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220

Example test name after:
check-kernel-oops-oops-bug-preempt-smp-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220

@bhcopeland bhcopeland requested a review from chaws July 23, 2024 09:54
@katieworton
Copy link
Member Author

@chaws @terceiro @nareshkamboju @roxell - If you can look over this PR and highlight if there are any issues with the proposed naming updates that would be great.

To keep this work on track, the PR should be merged this week - so if there are any obvious issues it would be great if you could highlight these ASAP.

Thanks!

@chaws
Copy link
Collaborator

chaws commented Jul 23, 2024

@chaws @terceiro @nareshkamboju @roxell - If you can look over this PR and highlight if there are any issues with the proposed naming updates that would be great.

To keep this work on track, the PR should be merged this week - so if there are any obvious issues it would be great if you could highlight these ASAP.

Thanks!

It is really well done :), but I found a couple of issues I commented in line. Also, please add a few tests to make sure the created naming matches the expected. Just like you did for test check-kernel-oops-oops-bug-preempt-smp-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220

@katieworton
Copy link
Member Author

@chaws @terceiro @nareshkamboju @roxell - If you can look over this PR and highlight if there are any issues with the proposed naming updates that would be great.
To keep this work on track, the PR should be merged this week - so if there are any obvious issues it would be great if you could highlight these ASAP.
Thanks!

It is really well done :), but I found a couple of issues I commented in line. Also, please add a few tests to make sure the created naming matches the expected. Just like you did for test check-kernel-oops-oops-bug-preempt-smp-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220

Thank you, @chaws!

I've made some updates based on the comment from @roxell: I moved the regex name up a level. So test called check-kernel-oops would now be called check-kernel-oops-oops-bug-preempt-smp (the version with the SHA would still be check-kernel-oops-oops-bug-preempt-smp-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220)

Happy to add more tests but let me know what you think of this change and we can go from there :)

@katieworton
Copy link
Member Author

@chaws - added some more unit tests for the SHA versions as I realised this was not dependent on the check-kernel-oops vs check-kernel-oops-oops-bug-preempt-smp change.

After speaking with @roxell, I also created a draft patch to try and change the naming from things like check-kernel-oops-oops-bug-preempt-smp to check-kernel-oops-bug-preempt-smp (removing the repetition): #1144

However, I had some issues with getting the unit tests going with #1144. I feel like we should just get this initial PR (#1142) merged then do another set of changes with improvements once we start seeing the outputs from SQUAD.

@roxell
Copy link
Contributor

roxell commented Jul 25, 2024

@chaws - added some more unit tests for the SHA versions as I realised this was not dependent on the check-kernel-oops vs check-kernel-oops-oops-bug-preempt-smp change.

After speaking with @roxell, I also created a draft patch to try and change the naming from things like check-kernel-oops-oops-bug-preempt-smp to check-kernel-oops-bug-preempt-smp (removing the repetition): #1144

We also talked about removing "check-kernel-oops-" completely.
@chaws is there any meaning of having "check-kernel-(oops|bug|warning|...) and having them "passing" if nothing if found?
Can't we just drop the "passing" tests all together?

However, I had some issues with getting the unit tests going with #1144. I feel like we should just get this initial PR (#1142) merged then do another set of changes with improvements once we start seeing the outputs from SQUAD.

@chaws
Copy link
Collaborator

chaws commented Jul 25, 2024

We also talked about removing "check-kernel-oops-" completely. @chaws is there any meaning of having "check-kernel-(oops|bug|warning|...) and having them "passing" if nothing if found? Can't we just drop the "passing" tests all together?

If I recall correctly, you were the one who asked for this :) so if you ask for removal let's just do it then

Add support for a test name regex to extract parts of the log parser
outputs to produce more meaningful test names.

Instead of producing test names with the "REGEX_NAME" + SHA, create test
names with "REGEX_NAME"+"REGEX_EXTRACT_NAME"+SHA to make the test names
easier to interpret.

Example test name before:
check-kernel-oops-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220

Example test name after:
check-kernel-oops-oops-bug-preempt-smp-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220

Signed-off-by: Katie Worton <[email protected]>
Move the meaningful name extracted by the REGEX_EXTRACT_NAME regex to be
included in the "higher-level" test name.

Before, test names would look like:
"check-kernel-oops"
And these would then be broken down into smaller test buckets with names
like:
"check-kernel-oops-oops-preempt-smp-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220"

Now, the higher-level test would be called:
"check-kernel-oops-oops-preempt-smp"
And the broken down tests would still look like:
"check-kernel-oops-oops-preempt-smp-a1acf2f0467782c9c2f6aeadb1d1d3cec136642b13d7231824a66ef63ee62220"

This makes the higher-level test names more meaningful, then if there
are differences which are not captured by the higher-level name, the SHA
can be used to differentiate the different cases.

Signed-off-by: Katie Worton <[email protected]>
Add an extra test case for multiple failures with SHAs.

Signed-off-by: Katie Worton <[email protected]>
Copy link
Collaborator

@chaws chaws left a comment

Choose a reason for hiding this comment

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

LGTM

@chaws chaws merged commit 84e9230 into Linaro:master Jul 25, 2024
7 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants