Re: [galaxy-dev] Verifying test output datatypes, was: Problem with change_format and conditional inputs?

2014-03-18 Thread John Chilton
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.c...@googlemail.com wrote:
 On Fri, Mar 14, 2014 at 9:04 PM, John Chilton jmchil...@gmail.com wrote:
 On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock p.j.a.c...@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
___
Please keep all replies on the list by using reply all
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:
  http://lists.bx.psu.edu/

To search Galaxy mailing lists use the unified search at:
  http://galaxyproject.org/search/mailinglists/


Re: [galaxy-dev] Verifying test output datatypes, was: Problem with change_format and conditional inputs?

2014-03-15 Thread Peter Cock
On Fri, Mar 14, 2014 at 9:04 PM, John Chilton jmchil...@gmail.com wrote:
 On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock p.j.a.c...@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
___
Please keep all replies on the list by using reply all
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:
  http://lists.bx.psu.edu/

To search Galaxy mailing lists use the unified search at:
  http://galaxyproject.org/search/mailinglists/


Re: [galaxy-dev] Verifying test output datatypes, was: Problem with change_format and conditional inputs?

2014-03-15 Thread Peter Cock
On Sat, Mar 15, 2014 at 12:50 PM, Peter Cock p.j.a.c...@googlemail.com wrote:
 On Fri, Mar 14, 2014 at 9:04 PM, John Chilton jmchil...@gmail.com wrote:
 On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock p.j.a.c...@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
___
Please keep all replies on the list by using reply all
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:
  http://lists.bx.psu.edu/

To search Galaxy mailing lists use the unified search at:
  http://galaxyproject.org/search/mailinglists/


Re: [galaxy-dev] Verifying test output datatypes, was: Problem with change_format and conditional inputs?

2014-03-14 Thread John Chilton
On Fri, Mar 14, 2014 at 10:24 AM, Peter Cock p.j.a.c...@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 jmchil...@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-attribute-if-defined-on-test/diff

 Thoughts?

 -John

 On Tue, Mar 11, 2014 at 10:30 AM, Peter Cock p.j.a.c...@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

___
Please keep all replies on the list by using reply all
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:
  http://lists.bx.psu.edu/

To search Galaxy mailing lists use the unified search at:
  http://galaxyproject.org/search/mailinglists/