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 Travis CI Shell Options #2490

Merged
merged 3 commits into from
May 28, 2020

Conversation

seldridge
Copy link
Member

This fixes two issues when using the new regression scripts on Travis infrastructure:

  1. travis_setup_env.bash may have an undefined variable TRAVIS_ENABLE_INFRA_DETECTION. This script needs to continue past this and have unset variable checking disabled.
  2. travis_wait.bash is written such that it needs to continue after the first failure to do useful things like print out the log of execution.

Related issue: None.

Type of change: bug fix

Impact: no functional change

Development Phase: implementation

Release Notes

None.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
@seldridge seldridge requested a review from richardxia May 27, 2020 17:08
Disables immediate failure when running travis_wait methods. These
tests need to keep going even if they fail to do things like print out
the log.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Copy link
Member

@richardxia richardxia left a comment

Choose a reason for hiding this comment

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

Ah, thanks for digging into this and fixing the issues! LGTM!

@seldridge
Copy link
Member Author

@richardxia: I have no idea what that failure was before I pushed. I assume that's spurious.

I did notice that the log for the JTAG tests is large (>500MiB). I added a commit that turns off +verbose, but was unsure if that was correct or would help with stability.

@richardxia
Copy link
Member

Yeah, I agree that it's probably spurious, since I can't see why any of the changes here would cause that test to specifically fail.

@seldridge
Copy link
Member Author

Ping @richardxia. Okay to merge this? (I technically have the power to do it, but that seems inappropriate.)

@richardxia
Copy link
Member

Ping @richardxia. Okay to merge this? (I technically have the power to do it, but that seems inappropriate.)

Yep, I think the build was still running when I left my review, but I can hit the merge button now. Thanks again!

@richardxia richardxia merged commit 711851d into chipsalliance:master May 28, 2020
@jackkoenig
Copy link
Contributor

Need that Mergify [Please Merge] label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants