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

test-setup.sh: outputs.zip creation may fail silently #8336

Closed
ghost opened this issue May 15, 2019 · 4 comments
Closed

test-setup.sh: outputs.zip creation may fail silently #8336

ghost opened this issue May 15, 2019 · 4 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Server Issues for serverside rules included with Bazel type: bug

Comments

@ghost
Copy link

ghost commented May 15, 2019

Description of the problem / feature request:

If the zip command from the following blurb fails for any reason, outputs.zip is not created and the user has no idea why.

# Zip up undeclared outputs.
if [[ -n "$TEST_UNDECLARED_OUTPUTS_ZIP" ]] && cd "$TEST_UNDECLARED_OUTPUTS_DIR"; then
(
shopt -s dotglob failglob
zip -qr "$TEST_UNDECLARED_OUTPUTS_ZIP" -- *
) 2> /dev/null
fi

This can happen if --

  • zip isn't installed on the test system;
  • the version of zip is too old and chokes on -- (stop laughing);
  • any other error occurs.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

$ touch WORKSPACE
$ cat > BUILD.bazel <<EOF
sh_test(name = "foo_test", srcs = ["foo_test.sh"])
EOF
$ cat > foo_test.sh <<"EOF"
#!/bin/bash
touch $TEST_UNDECLARED_OUTPUTS_DIR/foo
EOF
$ cat > zip <<EOF
#!/bin/bash
exit 1
EOF
$ chmod +x foo_test.sh zip
$ bazel test :foo_test --test_env=PATH=$PWD:/bin:/usr/bin

Observe that the test succeeds and bazel-testlogs/foo_test/test.outputs_manifest exists, but .../test.outputs/outputs.zip is missing.

What operating system are you running Bazel on?

  • CentOS 6.6
  • Debian 7.1

What's the output of bazel info release?

release 0.24.1+vmware

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

n/a

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

n/a

Have you found anything relevant by searching the web?

No.

Any other information, logs, or outputs that you want to share?

No.

@dslomov dslomov added team-Rules-Server Issues for serverside rules included with Bazel untriaged labels May 17, 2019
@hlopko hlopko added P2 We'll consider working on this in future. (Assignee optional) type: bug and removed untriaged labels Jun 7, 2019
@hlopko
Copy link
Member

hlopko commented Jun 7, 2019

@laszlocsomor is the person who's spent a lot of time in this area, maybe he could review the PR? :)

@laszlocsomor do you have plans to work on the test-setup.sh in the near term, maybe in the context of #5265?

@laszlocsomor
Copy link
Contributor

Thanks for the report and repro! Gentle facepalm at the culprit.

Currently I'm not planning to work on test-setup.sh, but I could quick-fix this (e.g. display warning message, ignore error). That'd let the test succeed -- failing to zip up the undeclared outputs is arguably a nuisance but no fatal flaw. How does that sound?

@ghost
Copy link
Author

ghost commented Jun 7, 2019

Currently I'm not planning to work on test-setup.sh, but I could quick-fix this (e.g. display warning message, ignore error). That'd let the test succeed -- failing to zip up the undeclared outputs is arguably a nuisance but no fatal flaw. How does that sound?

Sounds good to me. :)

@laszlocsomor laszlocsomor self-assigned this Jun 24, 2019
@laszlocsomor
Copy link
Contributor

I can't print a warning to the console because Bazel prints no output for passing tests. So I'll print the warning to the test log.

laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Jun 26, 2019
If test-setup.sh fails to create the undeclared
outputs zip, then print a warning but carry on.
Failing to create this zip is a nuisance but not a
fatal error.

The warning is printed to the test log. If the
test fails, this is printed to the console too. If
the test passes, then the warning remains only in
the test log (Bazel prints no output for passing
tests).

Fixes bazelbuild#8336

Change-Id: Iee8121d76e96445252d97142cef68c50afbb25b1
siberex pushed a commit to siberex/bazel that referenced this issue Jul 4, 2019
If test-setup.sh fails to create the undeclared
outputs zip, then print a warning but carry on.
Failing to create this zip is a nuisance but not a
fatal error.

The warning is printed to the test log. If the
test fails, this is printed to the console too. If
the test passes, then the warning remains only in
the test log (Bazel prints no output for passing
tests).

Fixes bazelbuild#8336

Change-Id: Iee8121d76e96445252d97142cef68c50afbb25b1

Closes bazelbuild#8720.

Change-Id: Iee8121d76e96445252d97142cef68c50afbb25b1
PiperOrigin-RevId: 255401321
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
If test-setup.sh fails to create the undeclared
outputs zip, then print a warning but carry on.
Failing to create this zip is a nuisance but not a
fatal error.

The warning is printed to the test log. If the
test fails, this is printed to the console too. If
the test passes, then the warning remains only in
the test log (Bazel prints no output for passing
tests).

Fixes bazelbuild#8336

Change-Id: Iee8121d76e96445252d97142cef68c50afbb25b1

Closes bazelbuild#8720.

Change-Id: Iee8121d76e96445252d97142cef68c50afbb25b1
PiperOrigin-RevId: 255401321
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Server Issues for serverside rules included with Bazel type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants