Skip to content

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Nov 18, 2020

While making recent changes to multiple-pipeline-capture.sh I had
vaguely noticed some similarities with multiple-pipeline-playback.sh but
I had naively not realized that they were both an exact copy/paste of
each other except for a couple lines. This commit make -playback catch
up with recent -capture enhancements. It also makes them nearly
identical again which is the first step before (painful) deduplication.

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

While making recent changes to multiple-pipeline-capture.sh I had
vaguely noticed some similarities with multiple-pipeline-playback.sh but
I had naively not realized that they were both an exact copy/paste of
each other except for a couple lines. This commit make -playback catch
up with recent -capture enhancements. It also makes them nearly
identical again which is the first step before (painful) deduplication.

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

marc-hb commented Nov 18, 2020

As of November 18 the version of the (newer) -capture test this catches up to didn't have a single nightly test run yet.

In other words this is not ready for merge yet.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 18, 2020

For https://sof-ci.01.org/softestpr/PR525/build412/devicetest/ APL_UP2_HDA was not available, everything else is PASS

@marc-hb marc-hb marked this pull request as ready for review November 18, 2020 23:25
@marc-hb marc-hb requested a review from a team as a code owner November 18, 2020 23:25
@marc-hb marc-hb marked this pull request as draft November 18, 2020 23:33
@marc-hb marc-hb marked this pull request as ready for review November 19, 2020 06:38
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 19, 2020

Except for the current shortage of APL_UP2_HDA , https://sof-ci.01.org/softestpr/PR525/build412/devicetest was successful.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 19, 2020

Once I'm all done I promise these tests will be all yours to deduplicate and clean-up! Appreciated.

These two test scripts are really almost exactly the same. After #525 is merged and successfully tested over some period of quiet (and rare) CI time, the easiest next step is probably to replace them both with one-line wrappers like this:

test-case/multiple-pipeline-playback.sh:

#!/bin/bash
multiple-pipeline-common.sh --prefer playback "$@"

test-case/multiple-pipeline-record.sh:

#!/bin/bash
multiple-pipeline-common.sh --prefer record "$@"

cc: @ranj063 , @aiChaoSONG

@xiulipan
Copy link
Contributor

@marc-hb Do you consider to merge this two scripts to one and use a simple option for different test?
migration could be done with

  • soft link like aplay/arecrod
  • tmp wrapper script

@xiulipan
Copy link
Contributor

These two test scripts are really almost exactly the same. After #525 is merged and successfully tested over some period of quiet (and rare) CI time, the easiest next step is probably to replace them both with one-line wrappers like this:

@marc-hb Seems we have same idea here.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 19, 2020

SOFCI TEST

EDIT: only to get a bit one additional test round and some extra coverage with the latest version of everything.
EDIT2: https://sof-ci.01.org/softestpr/PR525/build417/devicetest has no failure

Copy link

@aiChaoSONG aiChaoSONG left a comment

Choose a reason for hiding this comment

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

LGTM

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 20, 2020

OK let's fast-track this, I think it's low-risk and it's blocking a number of next steps.

@marc-hb marc-hb merged commit 310fed5 into thesofproject:master Nov 20, 2020
@marc-hb marc-hb deleted the multi-catchup branch November 20, 2020 05:47
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 24, 2020

Deduplication submitted in #532, not ready for merge today considering the other changes already merged today but 100% ready for review, thanks!

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