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

Add malformed tests check #177

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jtratner
Copy link

Currently, if you have a single quote in your test command, it'll break the bioconda build because of shell escaping. Not sure if the upstream will fix this (or if a fix is really worth it, given that you can always put your test in a test file), but the error is confusing to the user. This PR adds a single_quote_in_tests check to the linter.

Aside - I haven't been able to get the test suite to run yet (not sure how to let anaconda for python3 and python2 exist). But I'm hoping Travis will run 'em for me :)

@bgruening
Copy link
Member

Cool thanks @jtratner. xref: #163
I think the fix might be easier if we can get this to quote the parameters correctly: https://github.com/bioconda/bioconda-utils/blob/master/bioconda_utils/pkg_test.py#L134

@jtratner
Copy link
Author

@bgruening

I think the fix might be easier if we can get this to quote the parameters correctly: https://github.com/bioconda/bioconda-utils/blob/master/bioconda_utils/pkg_test.py#L134

yes, but also shell escaping gets so complicated that it makes my head hurt :-/ (so I picked the dumb way ;P)

@bgruening
Copy link
Member

That is perfectly fine for the time being! Thanks!

@jtratner
Copy link
Author

Test failures are unrelated... especially the ENCRYPTED_KEY_VAR for docs - wdyt @bgruening ?

@eseiler
Copy link
Member

eseiler commented Oct 19, 2017

Could the ENCRYPTED_KEY_VAR error be related to #167 ?

The current failure is due to running on a fork, which doesn't have the encrypted var available when cloning the repo over ssh. Should be a quick fix to use https when not on the main repo, I will fix this tomorrow.

Cloning via SSH was fixed after this PR, so you may need just to clone again.

@epruesse
Copy link
Member

epruesse commented Jan 8, 2019

@bgruening @jtratner What is the status of this? Do we still need this?

@bgruening
Copy link
Member

Looks still good to me. Could we have a rebase?

@epruesse
Copy link
Member

epruesse commented Jan 8, 2019

@bgruening - merged and conflicts resolved. It looks like it might be overzealous, but it is KISS.

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.

4 participants