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

Travis support #2

Merged
merged 7 commits into from
Aug 16, 2018
Merged

Travis support #2

merged 7 commits into from
Aug 16, 2018

Conversation

jayrbolton
Copy link
Contributor

No description provided.

@jayrbolton
Copy link
Contributor Author

@MrCreosote: If you have time, here is a PR to review. All this does is add a minimal .travis.yml that runs the tests and mucks around with some env vars. I wanted to get this working before working on all the complicated KBase deploy stuff.

@MrCreosote
Copy link
Member

I'm not really the right guy to review this stuff - I'm pretty hazy on how it all works. I'd recommend @sychan or @scanon.

@MrCreosote
Copy link
Member

I will note that if you use KB_AUTH_TOKEN to run tests, the tests will fail on PRs because they don't have access to that variable.

@sychan
Copy link
Contributor

sychan commented Aug 16, 2018

If you have a travis.yml KB_AUTH_TOKEN encrypted against the main kbase/CachingService and a separately defined one in another repo, I think it will work.

@sychan
Copy link
Contributor

sychan commented Aug 16, 2018

It looks good - coverage looks to be pretty good, and you're running the tests against the container that was built which is the ideal.

@sychan sychan merged commit 23166b1 into master Aug 16, 2018
@MrCreosote
Copy link
Member

@sychan but then wouldn't the other repo's token overwrite the main kbase repo on a PR merge?

@sychan
Copy link
Contributor

sychan commented Aug 16, 2018

Yes, you'd have to avoid committing changes to the travis.yml file. But something I should test is if you commit encrypted keys that are encrypted against another repo, does the build crash or just skip over it? In which case you can have the superset of all the necessary keys in your travis.yml

@MrCreosote
Copy link
Member

Yes, you'd have to avoid committing changes to the travis.yml file.

I'm missing something here - if you don't commit changes, how does your token get into the PR?

But something I should test is if you commit encrypted keys that are encrypted against another repo, does the build crash or just skip over it? In which case you can have the superset of all the necessary keys in your travis.yml

So everyone that needs to work with a repo, + 1 for the main repo, has an entry in the secure section?

Another alternative is finally getting auth working in minikb and just using that.

@sychan
Copy link
Contributor

sychan commented Aug 16, 2018

You're not missing anything, I wasn't thinking it through. Of course the PR would have to have to include the forked repo's keys, duh! I think mini-kb is the right long term solution, but we need to make sure all of the core services that a service requires for testing are available in mini-kb (I still need to convert another 4 services for full conversion) and then convert the existing unit tests to run against a configurable service instead of having it all buried in the test runner code.

I think Jay's approach of starting things up the test environment with docker compose is totally in line with the mini-kb approach.

@MrCreosote
Copy link
Member

Just auth + ws would cover a lot of scenarios. That's all the search and ujs need, for example. Search just configures the ws to use GFS for storing data instead of Shock. I think all the caching service & ID mapping service would need is auth.

@jayrbolton jayrbolton deleted the travis branch August 27, 2018 23:53
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