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

@xiulipan xiulipan commented Mar 30, 2020

Copy link
Copy Markdown
Contributor

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

@singalsu singalsu Mar 30, 2020

Copy link
Copy Markdown
Collaborator

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

@singalsu singalsu left a comment

Copy link
Copy Markdown
Collaborator

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

@lgirdwood lgirdwood left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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

xiulipan commented Mar 31, 2020

Copy link
Copy Markdown
Contributor Author

@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

xiulipan commented Mar 31, 2020

Copy link
Copy Markdown
Contributor Author

@singalsu @lgirdwood updated and Travis test passed.

@singalsu

singalsu commented Mar 31, 2020

Copy link
Copy Markdown
Collaborator

I just checked, testbench works for me too.

kv2019i
kv2019i approved these changes Mar 31, 2020
View reviewed changes

@kv2019i kv2019i left a comment

Copy link
Copy Markdown
Collaborator

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

@lgirdwood lgirdwood left a comment

Copy link
Copy Markdown
Member

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

jajanusz commented Apr 2, 2020

Copy link
Copy Markdown
Contributor

@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

lgirdwood commented Apr 3, 2020

Copy link
Copy Markdown
Member

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

marc-hb commented Apr 8, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@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

xiulipan commented Apr 9, 2020

Copy link
Copy Markdown
Contributor Author

@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.

@xiulipan xiulipan Apr 9, 2020

Copy link
Copy Markdown
Contributor Author

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

marc-hb commented Apr 9, 2020

Copy link
Copy Markdown
Collaborator

@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

@marc-hb marc-hb Aug 28, 2020

Copy link
Copy Markdown
Collaborator

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