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.

Sign up for GitHub

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

Jump to bottom

[tune] Fix and enable SigOpt tests #12877

Merged
merged 13 commits into from
Dec 18, 2020
Merged

[tune] Fix and enable SigOpt tests #12877

merged 13 commits into from
Dec 18, 2020

Conversation

krfricke
Copy link
Contributor

Why are these changes needed?

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@simon-mo
Copy link
Contributor

FYI Travis won't inject secrets to PR environment

@richardliaw
Copy link
Contributor

Looks like the sigopt tests were skipped

@krfricke
Copy link
Contributor Author

Ah thanks, I remember reading that somewhere. There's a couple of things here to fix, I'll get to that tomorrow and disable skip tests when SIGOPT_KEY is not found in the environment. Unfortunately that means we won't see if the tests pass on travis until we merged. (The SigOpt tests specifically pass locally, though).

@richardliaw richardliaw marked this pull request as ready for review December 16, 2020 22:01
richardliaw
richardliaw reviewed Dec 17, 2020
View reviewed changes
.travis.yml Outdated Show resolved Hide resolved
richardliaw
richardliaw reviewed Dec 17, 2020
View reviewed changes
.travis.yml Outdated Show resolved Hide resolved
@krfricke
Copy link
Contributor Author

Ah awesome, thanks!

richardliaw
richardliaw approved these changes Dec 17, 2020
View reviewed changes
Copy link
Contributor

@richardliaw richardliaw left a comment

Choose a reason for hiding this comment

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

Looks good now!

@richardliaw
Copy link
Contributor

(will wait one last time for tests to pass)

@krfricke krfricke added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Dec 18, 2020
@krfricke
Copy link
Contributor Author

The one failing test is unrelated to this PR - should be ready to merge!

@richardliaw richardliaw merged commit 55ae567 into ray-project:master Dec 18, 2020
@krfricke krfricke deleted the tune-sigopt-tests branch December 18, 2020 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardliaw richardliaw richardliaw approved these changes

Assignees
No one assigned
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@krfricke @simon-mo @richardliaw