Skip to content

Enable test bench test in simple scripts and test in Travis CI#2665

Merged
lgirdwood merged 2 commits into
thesofproject:masterfrom
xiulipan:pr/testbench
Apr 3, 2020
Merged

Enable test bench test in simple scripts and test in Travis CI#2665
lgirdwood merged 2 commits into
thesofproject:masterfrom
xiulipan:pr/testbench

Conversation

@xiulipan
Copy link
Copy Markdown
Contributor

@xiulipan xiulipan commented Mar 30, 2020

Update the host-testbench scripts to test 3 simple testbench
volume, src and eqiir.
Enable Travis CI test for the testbench with this script.

@xiulipan xiulipan requested a review from singalsu March 30, 2020 08:50
singalsu
singalsu reviewed Mar 30, 2020
View reviewed changes
Comment thread tools/test/audio/comp_run.sh Outdated
Copy link
Copy Markdown
Collaborator

@singalsu singalsu Mar 30, 2020

Choose a reason for hiding this comment

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

Oh, I see now that I've been generating the topologies into wrong directory. Thanks!

singalsu
singalsu approved these changes Mar 30, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

Good work, thanks!

Have run shellcheck (apt-get install shellcheck) for the bash scripts? It may help to find quotation issues etc. if there are any.

lgirdwood
lgirdwood approved these changes Mar 30, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@lgirdwood lgirdwood left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@xiulipan is it good to merge now ? I assume the test bench fail on Travis is known ?

@xiulipan
Copy link
Copy Markdown
Contributor Author

xiulipan commented Mar 31, 2020

@lgirdwood I haven't test the Travis scripts yet, it seem some issue with the test toplogy build here.
I will send a fix. @singalsu I think your fix for testbench is merged?

xiulipan added 2 commits March 31, 2020 10:20
@xiulipan
sripts: update host-testbench to work with latest code
14ab84e 
Enable 3 simple test with host-testbench:
volume, src and eqiir
Usage:
./scripts/build-tools.sh -t
./scripts/host-build-all.sh
./scripts/host-testbench.sh

Signed-off-by: Pan Xiuli <xiuli.pan@linux.intel.com>
@xiulipan
CI: travis: Add host-testbecnh
19be3c4 
build test topology.
builg host-testbench
Test with host-testbench.

Signed-off-by: Pan Xiuli <xiuli.pan@linux.intel.com>
@xiulipan
Copy link
Copy Markdown
Contributor Author

xiulipan commented Mar 31, 2020

@singalsu @lgirdwood updated and Travis test passed.

@singalsu
Copy link
Copy Markdown
Collaborator

singalsu commented Mar 31, 2020

I just checked, testbench works for me too.

kv2019i
kv2019i approved these changes Mar 31, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Excellent, thanks @xiulipan !

lgirdwood
lgirdwood approved these changes Mar 31, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@zrombel any idea why Internal CI would fail here, I dont think it tests the testbench ?

@jajanusz
Copy link
Copy Markdown
Contributor

jajanusz commented Apr 2, 2020

@zrombel any idea why Internal CI would fail here, I dont think it tests the testbench ?

I've reran this test, cos it looked like some random issue.

@lgirdwood
Copy link
Copy Markdown
Member

lgirdwood commented Apr 3, 2020

CI known issue.

@lgirdwood lgirdwood merged commit 1ecf435 into thesofproject:master Apr 3, 2020
@jajanusz jajanusz mentioned this pull request Apr 3, 2020
Merged
@marc-hb
Copy link
Copy Markdown
Collaborator

marc-hb commented Apr 8, 2020 •
edited
Loading

@singalsu , @xiulipan is there a plan for host-testbench.sh not to exit success 0 on failure? Until it does the interest of running it in CI seems limited.

$ ./scripts/host-testbench.sh | grep -i fail; echo $?
du: cannot access 'volume_out.raw': No such file or directory
volume test failed!
du: cannot access 'volume_out.raw': No such file or directory
eqiir test failed!

$ ./scripts/host-testbench.sh ; echo $?
0   #   <------

@xiulipan
Copy link
Copy Markdown
Contributor Author

xiulipan commented Apr 9, 2020

@marc-hb Good catch, I think I ignore some case here. Will set -e work here?

xiulipan
xiulipan commented Apr 9, 2020
View reviewed changes
Comment thread scripts/host-testbench.sh
#!/bin/bash
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2018 Intel Corporation. All rights reserved.

Copy link
Copy Markdown
Contributor Author

@xiulipan xiulipan Apr 9, 2020

Choose a reason for hiding this comment

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

@marc-hb Add set -e here?

@marc-hb
Copy link
Copy Markdown
Collaborator

marc-hb commented Apr 9, 2020

@xiulipan I filed #2752, we can discuss there.

In the mean time let's stop showing a green box in Travis: #2753

Have run shellcheck (apt-get install shellcheck) for the bash scripts?

Apparently not..

marc-hb
marc-hb reviewed Aug 28, 2020
View reviewed changes
Comment thread .travis.yml
env: PLATFORM=imx8m
- stage: testbench
script:
- ./scripts/docker-run.sh ./scripts/build-tools.sh -t &> /dev/null
Copy link
Copy Markdown
Collaborator

@marc-hb marc-hb Aug 28, 2020

Choose a reason for hiding this comment

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

Please never ever discard all output like this, especially not in CI where the exact configuration can be difficult to reproduce locally. If the script is too verbose then its verbosity can be adjusted but discarding everything is throwing the baby with the bathwater.

Fixed in #3364

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marc-hb marc-hb marc-hb left review comments

@lgirdwood lgirdwood lgirdwood approved these changes

@singalsu singalsu singalsu approved these changes

@kv2019i kv2019i kv2019i approved these changes

@dbaluta dbaluta Awaiting requested review from dbaluta

@jajanusz jajanusz Awaiting requested review from jajanusz

@lbetlej lbetlej Awaiting requested review from lbetlej

@mmaka1 mmaka1 Awaiting requested review from mmaka1

@plbossart plbossart Awaiting requested review from plbossart

@tlauda tlauda Awaiting requested review from tlauda

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@xiulipan @singalsu @jajanusz @lgirdwood @marc-hb @kv2019i