Skip to content

Commit

Permalink
runtime: Don't change test behaviour based on $CI or $KATA_DEV_MODE
Browse files Browse the repository at this point in the history
go-test.sh changes behaviour based on both the $CI and $KATA_DEV_MODE
variables, but not in a way that makes a lot of sense.

If either one is set it uses the test_coverage path, instead of the
test_local path.  That collects coverage information, as the name
suggests, but it also means it runs the tests twice as root and
non-root, which is very non-obvious.

It's not clear what use case the test_local path is for at all.
Developer local builds will typically have $KATA_DEV_MODE set and CI
builds will have $CI set.  There's essentially no downside to running
coverage all the time - it has little impact on the test runtime.

In addition, if *both* $CI and $KATA_DEV_MODE are set, the script
refuses to run things as root, considering it "unsafe".  While having
both set might be unwise in a general sense, there's not really any
way running sudo can be any more unsafe than it is with either one
set.

So, simplify everything by just always running the test_coverage path.
This leaves the test_local path unused, so we can remove it entirely.

Signed-off-by: David Gibson <[email protected]>
  • Loading branch information
dgibson committed May 13, 2022
1 parent 34c4ac5 commit cf465fe
Showing 1 changed file with 5 additions and 25 deletions.
30 changes: 5 additions & 25 deletions src/runtime/go-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,10 @@ test_coverage()
warn "As a result, only a subset of tests will be run"
warn "(run this script as a non-privileged to ensure all tests are run)."
else
if [ "$CI" = true ] && [ -n "$KATA_DEV_MODE" ]; then
warn "Dangerous to set CI and KATA_DEV_MODE together."
warn "NOT running tests as root."
else
# Run the unit-tests *twice* (since some must run as root and
# others must run as non-root), combining the resulting test
# coverage files.
users+=" root"
fi
# Run the unit-tests *twice* (since some must run as
# root and others must run as non-root), combining the
# resulting test coverage files.
users+=" root"
fi

echo "INFO: Currently running as user '$(id -un)'"
Expand All @@ -134,12 +129,6 @@ test_coverage()
done
}

# Run the tests locally
test_local()
{
eval go test "$go_test_flags" "$package"
}

main()
{
local long_option_names="${!long_options[@]}"
Expand Down Expand Up @@ -167,22 +156,13 @@ main()
shift
done

run_coverage=no
if [ "$CI" = true ] || [ -n "$KATA_DEV_MODE" ]; then
run_coverage=yes
fi

local go_ldflags
[ "$(go env GOARCH)" = s390x ] && go_ldflags="-extldflags -Wl,--s390-pgste"

# KATA_GO_TEST_FLAGS can be set to change the flags passed to "go test".
go_test_flags=${KATA_GO_TEST_FLAGS:-"-v $race -timeout $timeout_value -ldflags '$go_ldflags'"}

if [ "$run_coverage" = yes ]; then
test_coverage
else
test_local
fi
test_coverage
}

main "$@"

0 comments on commit cf465fe

Please sign in to comment.