Skip to content

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