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

Multi-playback/capture-pipeline always iterates in the same order #495

Open
ranj063 opened this issue Nov 5, 2020 · 9 comments
Open

Multi-playback/capture-pipeline always iterates in the same order #495

ranj063 opened this issue Nov 5, 2020 · 9 comments
Labels
P2 Critical bugs or normal features type:enhancement New framework feature or request type:test coverage gap This requires a new test case, not just fixing one

Comments

@ranj063
Copy link
Contributor

ranj063 commented Nov 5, 2020

The multi pipeline tests start and stop PCM in the same order. It doesnt cover the case where the PCMs are started in one order and stopped in another order. For example:
https://sof-ci.01.org/linuxpr/PR2371/build4845/devicetest/

In the above device test result, the multi-playback-pipeline.sh test passes on BYT/BDW/BSW because the PCMs are started in the order 0 followed by 1 and stopped in the order 0 followed by 1.

If I switch the order for stopping to 1 followed by 0, the test fails on my device.

cc:

@ranj063 ranj063 added type:bug Something doesn't work as expected type:enhancement New framework feature or request and removed type:bug Something doesn't work as expected labels Nov 5, 2020
@plbossart
Copy link
Member

Not following @ranj063 the test result link shows a failure for kmod load / unload.

Can you clarify how you made things fail?

@ranj063
Copy link
Contributor Author

ranj063 commented Nov 5, 2020

@plbossart yes, thatethe point. The multi-pipeline-playback.sh test for example on BYT starts PCM 0 followed by PCM 1. And stops PCM 0 followed by PCM 1.

But if you were to stop PCM1 before PCM0, it would have failed.

@plbossart
Copy link
Member

It still don't get how you found that out, do you have a patch for that script?

@marc-hb
Copy link
Collaborator

marc-hb commented Nov 5, 2020

If I switch the order for stopping to 1 followed by 0, the test fails on my device.

Seconding @plbossart : can you please submit a draft and never merged PR with that simple change?

A separate sof or linux github about the actual bug (as opposed to this test coverage gap) may also be needed.

@ranj063
Copy link
Contributor Author

ranj063 commented Nov 5, 2020

It still don't get how you found that out, do you have a patch for that script?

@plbossart I didnt use a script. On BYT, there are exactly 2 playback pipelines and luckily for me I was testing multi pipeline playback start and stop in that order and it failed.

@marc-hb What I am asking is not a one line change in the script especially with 4 active PCMs. I would have submitted the code if it were that simple

@marc-hb
Copy link
Collaborator

marc-hb commented Nov 5, 2020

I didnt use a script

You sound like you just entered some commands in a shell to reproduce this issue, no? So that is basically a script. bash is really not a great language (e.g. #312) but that's one of the advantages of using it for sof-test.

If what you did to reproduce is more complicated than entering a few commands then I think it's missing from the description.

I would have submitted the code if it were that simple

I did understand that a proper test solution wasn't easy, otherwise you would have submitted a PR and not this bug.

However compared to a proper solution it's often much easier to make some horrible, throw-away hacks in a test script purely to demonstrate some issue. Think of it as forcibly shoehorning the interactive commands you used into whichever script is the closest to what you want to do. Do not feel obligated by any coding standard. Considering how short and simple your test description sounds so far, I'm doubting we have no script remotely closed to what you did - but please correct me in the description.

@ranj063
Copy link
Contributor Author

ranj063 commented Nov 5, 2020

However compared to a proper solution it's often much easier to make some horrible, throw-away hacks in a test script purely to demonstrate some issue. Think of it as forcibly shoehorning the interactive commands you used into whichever script is the closest to what you want to do. Do not feel obligated by any coding standard. Considering how short and simple your test description sounds so far, I'm doubting we have no script remotely closed to what you did - but please correct me in the description.

@marc-hb i will attempt to make a script with what I have in my mind. stay tuned!

@aiChaoSONG
Copy link

I have add comments, and make it much clear, #563, close? @ranj063 @marc-hb

@marc-hb marc-hb added the good first issue Good for newcomers label Mar 5, 2021
@greg-intel
Copy link
Contributor

This is a pretty old ticket, and it's marked with "good first issue." Does Chao's PR close this ticket? I'm not sure I understand what the next steps are, here.

@marc-hb marc-hb removed the good first issue Good for newcomers label May 23, 2023
@marc-hb marc-hb added type:test coverage gap This requires a new test case, not just fixing one and removed type:coverage gap labels Jun 22, 2023
@marc-hb marc-hb changed the title Multi-playback/capture-pipeline test is not comprehensive Multi-playback/capture-pipeline always iterates in the same order Jun 22, 2023
@marc-hb marc-hb added the P2 Critical bugs or normal features label Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Critical bugs or normal features type:enhancement New framework feature or request type:test coverage gap This requires a new test case, not just fixing one
Projects
None yet
Development

No branches or pull requests

5 participants