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

Fix the smoke tests #11844

Merged
merged 4 commits into from Jul 20, 2020
Merged

Fix the smoke tests #11844

merged 4 commits into from Jul 20, 2020

Conversation

SimonRichardson
Copy link
Member

@SimonRichardson SimonRichardson commented Jul 20, 2020

Description of change

The make go-build has changed from 2.7 to 2.8 and the smoke tests
previously checked if there was any output.

The following updates the output from the integration tests to be much
more simplistic. There are now only two modes, the very terse mode,
which is great to just see from a very high-level overview if something
works or broke:

./main.sh smoke

Or using the verbose mode that aims to be chatty, but not overly chatty
like with the use of set -eux. The x part makes it hard to see
what's happening and should be only used for local debugging.

./main.sh -v smoke

With this simplistic model, we should be able to see what's failing
within jenkins, as we use -v as the default mode for all our testing.

QA steps

cd tests && ./make smoke

The `make go-build` has changed from 2.7 to 2.8 and the smoke tests
previously checked if there was any output. Now we check for if the
command was successful. As we need to use the `$?` pattern a lot I've
added it to the SHELLCHECK_OPTS.
tests/main.sh Outdated Show resolved Hide resolved
tests/suites/smoke/build.sh Outdated Show resolved Hide resolved
Additionally we need to see output from some commands to help diagnose
issues. You can do this with the switch statement.

I'm currently unsure about keeping VERBOSE 3 level, as it's so chatty
nobody can tell what's going on... we really need to re-think that part.
echo "\\n${OUT}"
exit 1
fi
make go-build 2>&1
Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than be fancy, just let it fail!

Copy link
Contributor

Choose a reason for hiding this comment

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

if you still want to be fancy:

OUT=$(make go-build 2>&1 || echo "BUILD FAILED")
if [ "$OUT" =~ (BUILD FAILED) ]; then
  ....
fi

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll think about it

tests/includes/run.sh Outdated Show resolved Hide resolved
The following updates the output from the integration tests to be much
more simplistic. There are now only two modes, the very terse mode,
which is great to just see from a very high level overview if something
works or broke:

   ./main.sh smoke

Or using the verbose mode that aims to be chatty, but not overly chatty
like with the use of `set -eux`. The `x` part makes it hard to see
what's happening and should be only used for local debugging.

   ./main.sh -v smoke

With this simplistic model we should be able to see what's failing
within jenkins, as we use `-v` as the default mode for all our testing.
Copy link
Contributor

@achilleasa achilleasa left a comment

Choose a reason for hiding this comment

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

Nice work! You may also want to update the PR description to highlight the cleanup work in output handling

fi

while read data; do
if [ -z "${output}" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

It did take me a while to figure out what the various if blocks do here. It would be great if you could add some comments for others examining this part of the code in the future.

The output function is a bit cryptic, so I've added some comments to
help understand that.

The output function aims to solve all the outputing issues from running
commands against juju.
@SimonRichardson
Copy link
Member Author

$$merge$$

1 similar comment
@SimonRichardson
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit 09df46c into juju:2.8 Jul 20, 2020
jujubot added a commit that referenced this pull request Jul 22, 2020
#11852

Zero-conflict merge from 2.8 to bring forward:
- #11851 from manadart/2.8-remove-lld-refcount
- #11846 from howbazaar/2.8-watch-models-application-status
- #11840 from ycliuhw/fix/lp-1875481
- #11845 from ycliuhw/fix/lp-1883006
- #11844 from SimonRichardson/fix-smoke-tests
- #11837 from tlm/pod-spec-json
@SimonRichardson SimonRichardson deleted the fix-smoke-tests branch July 12, 2021 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants