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

test-case: add missing pipeline count acquisition logic #380

Merged
merged 1 commit into from
Sep 21, 2020

Conversation

aiChaoSONG
Copy link

@aiChaoSONG aiChaoSONG commented Sep 17, 2020

The commit 84f5895 wrongly remove the code logic to
acquire pipeline count from multi-pipeline-playback/
capture test cases. This leads to the 'max_count' used
in these scripts being empty, as no pipeline is actually
tested, the test cases will always pass.

This patch adds the pipeline count acquisition logic back.

@aiChaoSONG aiChaoSONG requested a review from a team as a code owner September 17, 2020 06:43
@marc-hb
Copy link
Collaborator

marc-hb commented Sep 18, 2020

Since #373 was merged I see that multiple-pipeline tests have an empty trace like this:

2020-09-15 00:00:31 UTC [REMOTE_INFO] nlines=1 /home/ubuntu/sof-test/logs/multiple-pipeline-capture/2020-09-15-00:00:20-8571/slogger.txt

This will become a FAIL once #297 is fully implemented.

@marc-hb
Copy link
Collaborator

marc-hb commented Sep 18, 2020

This looks like a partial revert of 84f5895 / PR #281. If it is then it should be mentioned in the commit message.

@aiChaoSONG
Copy link
Author

@marc-hb thanks, will update commit message to mention 84f5895

@marc-hb
Copy link
Collaborator

marc-hb commented Sep 18, 2020

Since #373 was merged I see that multiple-pipeline tests have an empty trace like this:

Forgot to mention it's intermittently empty on my Up2 board. This fix does not look like a fix for a non-deterministic issue...

@aiChaoSONG what's your logger experience when running the multipipeline tests? Before and after this PR?

UPDATE after a bit more testing: not intermittently empty: empty the second time I run ./test-case/multiple-pipeline-playback.sh. It's probably a different issue.

@aiChaoSONG
Copy link
Author

@marc-hb Before this PR, the test case always PASS, even the topology not loaded(should fail), and the terminal message like this:

2020-09-18 05:13:38 UTC [INFO] Starting /usr/bin/sof-logger -t -l /etc/sof/sof-cml.ldc -o /home/ubuntu/Downloads/sof-test/logs/multiple-pipeline-playback/2020-09-18-13:13:38-30729/slogger.txt
2020-09-18 05:13:38 UTC [INFO] ===== Testing: (Loop: 1/1) =====
2020-09-18 05:13:38 UTC [INFO] pipeline start sleep 0.5s for device wakeup
2020-09-18 05:13:43 UTC [INFO] checking pipeline status
2020-09-18 05:13:43 UTC [INFO] preparing sleep 5
2020-09-18 05:13:48 UTC [INFO] checking pipeline again
2020-09-18 05:13:49 UTC [INFO] nlines=1 /home/ubuntu/Downloads/sof-test/logs/multiple-pipeline-playback/2020-09-18-13:13:38-30729/slogger.txt
2020-09-18 05:13:50 UTC [INFO] Test Result: PASS!

After this PR, if topology loaded, terminal message looks like this:
terminal_msg.txt
if topology load failed, terminal message looks like this:
terminal_msg.txt

In fact, before this PR, no pipeline is checked, that's why it always pass.

The commit 84f5895 wrongly remove the code logic to
acquire pipeline count from multi-pipeline-playback/
capture test cases. This leads to the 'max_count' used
in these scripts being empty, as no pipeline is actually
tested, the test cases will always pass.

This patch adds the pipeline count acquisition logic back.

Signed-off-by: Amery Song <chao.song@intel.com>
Copy link

@keyonjie keyonjie left a comment

Choose a reason for hiding this comment

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

Thanks for spotting it.

@aiChaoSONG
Copy link
Author

merge for tgl release

Copy link
Collaborator

@fredoh9 fredoh9 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix

@aiChaoSONG aiChaoSONG merged commit b220876 into thesofproject:master Sep 21, 2020
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.

4 participants