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 s3 specs - part 2 #1539

Merged
merged 1 commit into from
Oct 3, 2018
Merged

Fix s3 specs - part 2 #1539

merged 1 commit into from
Oct 3, 2018

Conversation

orangewolf
Copy link
Member

As hypothesized, the specs are not running in master any more. That appears to be because TRAVIS_PULL_REQUEST_SLUG if you are not talking about a pull request. This should work in both cases now.

@orangewolf
Copy link
Member Author

I'll check master again once this is merged to make sure the AWS specs are running there again.

config.filter_run_excluding(aws: true) unless ENV['CI'] && ENV['TRAVIS_PULL_REQUEST_SLUG'].match('samvera-labs/hyku')
# is where the valid aws keys live. TRAVIS_PULL_REQUEST_SLUG is "" when the job is a push job
unless ENV['CI'] &&
(ENV['TRAVIS_PULL_REQUEST_SLUG'].match('samvera-labs/hyku') || ENV['TRAVIS_PULL_REQUEST_SLUG'].blank?)
Copy link
Member

Choose a reason for hiding this comment

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

In this case, could you just remove the TRAVIS_PULL_REQUEST_SLUG entirely? I'm not understanding what situation this is trying to prevent. Maybe you could modify the commit message to explain a bit more.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jcoyne here is the description from part 1. The issue is that only the travis ci from this repo is set to run the S3 specs successfully. So all forked pull request spec sets fail on the S3 connection tests. BUT we want those test to run after a branch is merged to make sure there is no regression. This commit prevents the tests from running locally (as it was before) and from running on forks, but makes sure they run on pull requests from within this repo and on merged branches. This is part 2, simply because I can't test that the specs actually run like they are supposed to until it is merged it is a little tricky to know if we got the variables right for all the cases w/o getting a merge =-)


Fixes #1524

No longer run S3 related specs on non-samvera-labs repos. This will allow specs to pass on forks.

This is a first swing at getting Travis to not run the specs unless the code is in samvera-labs/hyku. Specs on forks will thus pass and the 4 failing S3 specs from forks will get run properly once code is merged.

Note - need to check that the specs in question are run after this is merged.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. 👍

@jcoyne
Copy link
Member

jcoyne commented Jul 26, 2018

Do you want to squash these commits together?

@orangewolf
Copy link
Member Author

@jcoyne finally got back to squashing this.

Copy link
Member

@cjcolvar cjcolvar 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 to me.

@cjcolvar cjcolvar merged commit a93a8f3 into samvera:master Oct 3, 2018
@ghost ghost mentioned this pull request Oct 4, 2018
@orangewolf orangewolf deleted the fix_s3_specs branch October 4, 2018 17:36
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.

None yet

3 participants