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 false positive on schema validation #290

Closed
jamesaoverton opened this issue Jan 12, 2017 · 13 comments
Closed

Travis false positive on schema validation #290

jamesaoverton opened this issue Jan 12, 2017 · 13 comments
Labels

Comments

@jamesaoverton
Copy link
Member

Reported by @alanruttenberg in #287. The geno.yml file did not include the required term_browser key, which caused make validate to fail in Alan's local environment. Validation also failed on the PURL server, blocking deployment (which is good), but nobody noticed until Alan manually checked his IAO changes #286.

However all tests passed on Travis (https://travis-ci.org/OBOFoundry/purl.obolibrary.org/builds/190343322). I'm very unhappy about this, because I was trusting Travis to prevent bad merges.

@jamesaoverton
Copy link
Member Author

I just found another one of these: a duplicate entry passed validation on Travis but not on the server. Fixed by #379.

Now I see that kwalify is failing on Travis without causing the build to fail: bash: kwalify: command not found https://travis-ci.org/OBOFoundry/purl.obolibrary.org/builds/302132357#L577 It looks like this was the case for the original bug report.

Working on this in #380.

@jamesaoverton
Copy link
Member Author

Well, now we know three things that don't work...

@jamesaoverton
Copy link
Member Author

Ran into this again today. :(

@kltm
Copy link
Contributor

kltm commented Jul 3, 2018

@jamesaoverton If I'm reading this correctly, I think this can be easily worked around. You may want to look at the travis file we use for reusable data:
https://github.com/reusabledata/reusabledata/blob/master/.travis.yml

@jamesaoverton
Copy link
Member Author

Thanks @kltm. I would very much like to see this fixed, but I’m going on vacation for a few weeks. If you have time to try the fix, that would be great.

@kltm
Copy link
Contributor

kltm commented Jul 3, 2018

@jamesaoverton At this point, I'm fairly unfamiliar with how the site/testing hangs together, but I should be able to bug @cmungall enough to try a PR.

@cmungall
Copy link
Contributor

cmungall commented Jul 3, 2018

@kltm - it's all self-contained ansible so you should be able to get it running without knowing the ins and outs

Let me see if I understand the issue first, I'll try and restate

The validate step requires kwalify. If kwalify is not found, then validation auto-passes. This is not good as it leads to different behavior between travis and production.

An example from the above build where kwalify silently fails on Travis:

72.72s$ sudo make all test
rm -rf temp tests
kwalify -f tools/config.schema.yml config/*.yml \
	| awk '{print} /INVALID/ {status=1} END {exit status}'
bash: kwalify: command not found
mkdir backup
mkdir -p temp/obo/aeo

This is unusual as we expect the build to fail if kwalify is not found. We should fix the makefile
https://github.com/OBOFoundry/purl.obolibrary.org/blob/master/Makefile#L72-L74

Piping the output of kwalify masks the exit code. There are different ways to do get around. I would just split into two targets.

Other suggestions here:

https://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another
https://stackoverflow.com/questions/23079651/equivalent-of-pipefail-in-gnu-make

Fixing this will solve the immediate issue in that if the environment is not as expected and kwalify is not found then travis will fail, preventing us accidentally merging in bad syntax and messing with the server. I can make the PR for this.

However, we want to avoid any future problems by guaranteeing that the travis environment and the server environment are identical, including ensuring both have kwalify. I'm not familiar with ansible.

It seems that we do explicitly include it:

- name: Install kwalify
command: gem install kwalify creates=/usr/local/bin/kwalify

And the offending build did install it:
https://travis-ci.org/OBOFoundry/purl.obolibrary.org/builds/302132357#L528-L529

So why the same build later fails to find it puzzles me. Any thoughts @kltm?

In any case, we should be ultra-careful merging in PURL PRs while James is away. I can fix the Makefile but may wait til James is back before merging.

@cmungall
Copy link
Contributor

cmungall commented Jul 3, 2018

@kltm

You may want to look at the travis file we use for reusable data:
https://github.com/reusabledata/reusabledata/blob/master/.travis.yml

This seems a bit more straightforward as all that Travis does is run kwalify so we can do in a ruby environment. For many other repos we tend to use python or java as base environment (mixing in ruby can be hard). But for this repo, the setup is all down to ansible.

@jamesaoverton
Copy link
Member Author

@cmungall: Good summary. Yes, the main problem is that I can’t find kwalify inside Travis. It works fine in other environments using the same Ansible scripts, so it’s hard to debug.

@cmungall
Copy link
Contributor

cmungall commented Jul 3, 2018 via email

@kltm
Copy link
Contributor

kltm commented Jul 3, 2018

@cmungall I would say likely a path issue; I'd dump the user PATH or use the full bin path to make sure.

In travis at least, the distinction between different images is pretty arbitrary: they are all fairly standard ubuntu images, for good and ill, and can be changed easily--you can see that I run both ruby/kwalify and node tests in my travis without any issue.

@jamesaoverton
Copy link
Member Author

Yes It’s probably a path issue, I just gave up before I figured it out. See my failed PRs #380 #381 #382.

Thanks for looking into this. I’m supposed to be on vacation now, so I’ll leave you to it.

@jamesaoverton
Copy link
Member Author

In #494 we replaced kwalify with JSON schema (also used for the OBO registry).

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

No branches or pull requests

3 participants