Conda-build: Testing should better isolate tests rather than using &&.

[dpryan79]

Actual Behavior

Xref: https://github.com/bioconda/bioconda-recipes/pull/16873

In short, if you have tests that include || then preceding tests can fail but not causing the testing to fail overall.

Expected Behavior

A failure of one test should cause failure, regardless of what comes after it. One option would be to just enclose each test command in parentheses.

Steps to Reproduce

See https://github.com/bioconda/bioconda-recipes/pull/16873

[marcelotrevisani]

I don't think that is a bug or an improvement at least not for now
Because it will evaluate those commands as bash commands

bash command which uses the || will evaluate the first condition, if it is true it will stop. In your case, if the command is successful it will stop, otherwise, it will evaluate the next one, it will fail just if all of them fail.
It is like in any other programming language. For example in Python:

if False or True or False or func_does_not_exist():
    print('SUCCESS')

It will print Success and the code will exit successfully

[marcelotrevisani]

I will add @msarahan to this discussion. But for me, this is the correct behavior

[marcelotrevisani]

For your case, if you need something a bit more complex you can write a script, like a bash script and call it using the commands

[msarahan]

This could possibly be accomplished by prepending set -e to build scripts. I'm not really opposed to that, but it could have some annoying unanticipated consequences.

[bernt-matthias]

Lets say I have two tests:

• X
• Y or Z

X and Y are false and Z is true

Taken separately the 1st test fails and the second one is successful.
But taken together the tests are successful, because conda just chains them together using &&, i.e. X && Y or Z which evaluates to true.

I agree that its not really a bug, but IMHO the behavior would be more intuitive if conda would add parentheses around each test in order to fix the evaluation order, i.e. (X) && (Y or Z). I don't see any side effects for this strategy.

I'm not sure if set -e would help here.

[marcelotrevisani]

Now I see what you are talking about! Sorry about that!
Thanks for the explanation!
Now I agree that would be good to put brackets between each command

[marcelotrevisani]

I created a small recipe to test those things. However, it is working as expected
Basically, those commands in the test section will be put in a script called run_test.sh which already has the option set -ex and that script will be running with -e option as well.

The bare minimum recipe which I created is:

package:
  name: test_cmds_logic
  version: 1.0

test:
  commands:
    - cmd_does_not_exist
    - cmd_does_not_exist || echo "TEST"

and this is failing right on the first line as expected.

Am I missing something here?
@bernt-matthias @dpryan79

[dpryan79]

Perhaps something has changed in how conda does this, as the bioconda PR I referenced passed testing rather than failing in exactly this scenario (that or perl returns with 0 when it aborts due to an error, which seems unlikely).

[bernt-matthias]

Can you see the exact command that is executed? In the bioconda CI logs I found the following line:

15:37:54 BIOCONDA INFO (ERR) [Aug 12 15:37:54] STEP Run image [quay.io/biocontainers/trinity:2.8.5--h8b12597_1] with command [[Trinity --cite && align_and_estimate_abundance.pl --help || [[ "$?" == 255 ]] && abundance_estimates_to_matrix.pl --help || [[ "$?" == 255 ]] && run_DE_analysis.pl -h || [[ "$?" == 255 ]] && analyze_diff_expr.pl -h || [[ "$?" == 255 ]] && PtR --help || [[ "$?" == 255 ]] && TrinityStats.pl 2>&1 | grep "usage:" && define_clusters_by_cutting_tree.pl 2>&1 | grep "define K clusters via k-means algorithm" && Trinity_gene_splice_modeler.py --help]]

or perl returns with 0 when it aborts due to an error, which seems unlikely.

I think we can exclude this since we test for [[ "$?" == 255 ]]

[bernt-matthias]

Seems to be the test for the docker container.

[dpryan79]

Hmm, the example recipe from @marcelotrevisani also fails in the same docker container with the same conda version, so something more complicated must have been going on with that trinity recipe.

[marcelotrevisani]

I am going to close this issue since it does not seem to be related to conda-build.
However, if you find anything which might be related to it feel free to reopen this issue or to create a new one.

Thanks for reporting it anyway! :)

[dpryan79]

Seems reasonable, thanks @marcelotrevisani

[dpryan79]

@bernt-matthias I'm working on a different PR now, but after that I'd like to look into this more.

[dpryan79]

Oooh, I think this is an issue on our (bioconda's) side in how the mulled testing is done. Conda is totally innocent in this issue :)