Verifying test output datatypes, was: Problem with <change_format> and conditional inputs?
Thanks John, I suggest making this test framework perform this check by default (the twill and API based frameworks) and seeing what - if anything - breaks as a result on the Test Tool Shed. Note that one area of fuzziness is subclasses, e.g. if the tool output was labelled "fastqsanger", but the <test> just said "fastq", I would say the test was broken. On the other hand, if the <test> used a specific datatype like "fastqsanger" but the tool produced a dataset tagged with a more generic datatype like "fastq" I think that is a again a real failure. Peter On Fri, Mar 14, 2014 at 3:15 PM, John Chilton <jmchilton@gmail.com> wrote:
Grepping around the code I think this is the only way a ftype attribute on an output affects the evaluation of test data.
if attributes.get( 'ftype', None ) == 'bam': local_fh, temp_name = self._bam_to_sam( local_name, temp_name ) local_name = local_fh.name
I am not sure it was ever meant as a strict test.
I worry about breaking backward compatibility but it is easy enough to implement this as an actual check when using newer API driven tests. I have opened a pull request for this functionality here:
https://bitbucket.org/galaxy/galaxy-central/pull-request/347/check-ftype-att...
Thoughts?
-John
On Tue, Mar 11, 2014 at 10:30 AM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
That seems to work - thanks Björn :)
This seems to have exposed a bug in the <test> framework, e.g.
<test> <param name="query" value="rhodopsin_nucs.fasta" ftype="fasta" /> <param name="db_opts_selector" value="file" /> <param name="subject" value="three_human_mRNA.fasta" ftype="fasta" /> <param name="database" value="" /> <param name="evalue_cutoff" value="1e-40" /> <param name="out_format" value="5" /> <param name="adv_opts_selector" value="basic" /> <output name="output1" file="blastn_rhodopsin_vs_three_human.xml" ftype="blastxml" /> </test>
This was saying the output should have been "blastxml", but until I just fixed it the output was being tagged as "tabular" (although run_functional_tests.sh did check the content it didn't check the datatype matched).
Dave - do think this is a reasonable enhancement?
Peter
On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
Thanks John,
I suggest making this test framework perform this check by default (the twill and API based frameworks) and seeing what - if anything - breaks as a result on the Test Tool Shed.
Hey Peter, I hope it is okay, but I do not want to make this change to the Twill driven variant of tool tests, I consider that code at end of life - new development would be a waste I think. Running all tools against a modified environment that switched all tests to target the APIs would be nice, but it sounds like there is not really the infrastructure in place for doing that right now. Upon further consideration I am not sure there are really any backward compatibility concerns anyway - or at least no more so than anything else when switching over to the API driven tests. I'll let the pull request sit open for a few days and then merge it as is.
Note that one area of fuzziness is subclasses, e.g. if the tool output was labelled "fastqsanger", but the <test> just said "fastq", I would say the test was broken. On the other hand, if the <test> used a specific datatype like "fastqsanger" but the tool produced a dataset tagged with a more generic datatype like "fastq" I think that is a again a real failure.
100% agreed on both points. I believe the implementation proposed in pull request #347 reflects this resolution of the "fuzziness". Thanks and have a great weekend, -John
Peter
On Fri, Mar 14, 2014 at 3:15 PM, John Chilton <jmchilton@gmail.com> wrote:
Grepping around the code I think this is the only way a ftype attribute on an output affects the evaluation of test data.
if attributes.get( 'ftype', None ) == 'bam': local_fh, temp_name = self._bam_to_sam( local_name, temp_name ) local_name = local_fh.name
I am not sure it was ever meant as a strict test.
I worry about breaking backward compatibility but it is easy enough to implement this as an actual check when using newer API driven tests. I have opened a pull request for this functionality here:
https://bitbucket.org/galaxy/galaxy-central/pull-request/347/check-ftype-att...
Thoughts?
-John
On Tue, Mar 11, 2014 at 10:30 AM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
That seems to work - thanks Björn :)
This seems to have exposed a bug in the <test> framework, e.g.
<test> <param name="query" value="rhodopsin_nucs.fasta" ftype="fasta" /> <param name="db_opts_selector" value="file" /> <param name="subject" value="three_human_mRNA.fasta" ftype="fasta" /> <param name="database" value="" /> <param name="evalue_cutoff" value="1e-40" /> <param name="out_format" value="5" /> <param name="adv_opts_selector" value="basic" /> <output name="output1" file="blastn_rhodopsin_vs_three_human.xml" ftype="blastxml" /> </test>
This was saying the output should have been "blastxml", but until I just fixed it the output was being tagged as "tabular" (although run_functional_tests.sh did check the content it didn't check the datatype matched).
Dave - do think this is a reasonable enhancement?
Peter
On Fri, Mar 14, 2014 at 9:04 PM, John Chilton <jmchilton@gmail.com> wrote:
On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
Thanks John,
I suggest making this test framework perform this check by default (the twill and API based frameworks) and seeing what - if anything - breaks as a result on the Test Tool Shed.
Hey Peter,
I hope it is okay, but I do not want to make this change to the Twill driven variant of tool tests, I consider that code at end of life - new development would be a waste I think.
Running all tools against a modified environment that switched all tests to target the APIs would be nice, but it sounds like there is not really the infrastructure in place for doing that right now.
Upon further consideration I am not sure there are really any backward compatibility concerns anyway - or at least no more so than anything else when switching over to the API driven tests. I'll let the pull request sit open for a few days and then merge it as is.
Note that one area of fuzziness is subclasses, e.g. if the tool output was labelled "fastqsanger", but the <test> just said "fastq", I would say the test was broken. On the other hand, if the <test> used a specific datatype like "fastqsanger" but the tool produced a dataset tagged with a more generic datatype like "fastq" I think that is a again a real failure.
100% agreed on both points. I believe the implementation proposed in pull request #347 reflects this resolution of the "fuzziness".
Thanks and have a great weekend, -John
That sounds like a plan :) Hmm. I wonder if it would be trivial to tweak our TravisCI setup to run the functional tests twice, once with the old twill framework and once with the new API based framework? Seems doable (but would up the run time quite a bit). Thanks, Peter
On Sat, Mar 15, 2014 at 12:50 PM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
On Fri, Mar 14, 2014 at 9:04 PM, John Chilton <jmchilton@gmail.com> wrote:
On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
Thanks John,
I suggest making this test framework perform this check by default (the twill and API based frameworks) and seeing what - if anything - breaks as a result on the Test Tool Shed.
Hey Peter,
I hope it is okay, but I do not want to make this change to the Twill driven variant of tool tests, I consider that code at end of life - new development would be a waste I think.
Running all tools against a modified environment that switched all tests to target the APIs would be nice, but it sounds like there is not really the infrastructure in place for doing that right now.
Upon further consideration I am not sure there are really any backward compatibility concerns anyway - or at least no more so than anything else when switching over to the API driven tests. I'll let the pull request sit open for a few days and then merge it as is.
Note that one area of fuzziness is subclasses, e.g. if the tool output was labelled "fastqsanger", but the <test> just said "fastq", I would say the test was broken. On the other hand, if the <test> used a specific datatype like "fastqsanger" but the tool produced a dataset tagged with a more generic datatype like "fastq" I think that is a again a real failure.
100% agreed on both points. I believe the implementation proposed in pull request #347 reflects this resolution of the "fuzziness".
Thanks and have a great weekend, -John
That sounds like a plan :)
Hmm. I wonder if it would be trivial to tweak our TravisCI setup to run the functional tests twice, once with the old twill framework and once with the new API based framework? Seems doable (but would up the run time quite a bit).
I've just seen your other email, the new environment variable GALAXY_TEST_DEFAULT_INTERACTOR would make this easy. http://lists.bx.psu.edu/pipermail/galaxy-dev/2014-March/018673.html http://dev.list.galaxyproject.org/Tool-Testing-Enhancements-tt4663799.html Peter
Merged. Thanks again for the input! I will look into the unicode issue and respond on the other thread. -John On Sat, Mar 15, 2014 at 7:50 AM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
On Fri, Mar 14, 2014 at 9:04 PM, John Chilton <jmchilton@gmail.com> wrote:
On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock <p.j.a.cock@googlemail.com> wrote:
Thanks John,
I suggest making this test framework perform this check by default (the twill and API based frameworks) and seeing what - if anything - breaks as a result on the Test Tool Shed.
Hey Peter,
I hope it is okay, but I do not want to make this change to the Twill driven variant of tool tests, I consider that code at end of life - new development would be a waste I think.
Running all tools against a modified environment that switched all tests to target the APIs would be nice, but it sounds like there is not really the infrastructure in place for doing that right now.
Upon further consideration I am not sure there are really any backward compatibility concerns anyway - or at least no more so than anything else when switching over to the API driven tests. I'll let the pull request sit open for a few days and then merge it as is.
Note that one area of fuzziness is subclasses, e.g. if the tool output was labelled "fastqsanger", but the <test> just said "fastq", I would say the test was broken. On the other hand, if the <test> used a specific datatype like "fastqsanger" but the tool produced a dataset tagged with a more generic datatype like "fastq" I think that is a again a real failure.
100% agreed on both points. I believe the implementation proposed in pull request #347 reflects this resolution of the "fuzziness".
Thanks and have a great weekend, -John
That sounds like a plan :)
Hmm. I wonder if it would be trivial to tweak our TravisCI setup to run the functional tests twice, once with the old twill framework and once with the new API based framework? Seems doable (but would up the run time quite a bit).
Thanks,
Peter
participants (2)
-
John Chilton -
Peter Cock