fix-fix-1971 #2001
fix-fix-1971 #2001
Conversation
|
Is it possible to add a test? |
|
Not sure, suggestions? |
| @@ -173,7 +173,8 @@ def study_prep_get_req(study_id, user_id): | |||
| info['start_artifact_id'] = start_artifact.id | |||
| info['youngest_artifact'] = '%s - %s' % ( | |||
| youngest_artifact.name, youngest_artifact.artifact_type) | |||
| info['ebi_experiment'] = bool(prep.ebi_experiment_accessions) | |||
| info['ebi_experiment'] = bool( | |||
There was a problem hiding this comment.
I am likely missing something, this is going to generate a list of "accessions", and if some are None, they will still be included in the resulting list, right? There's no filter being applied here. Otherwise, what's the goal of enumerating explicitly the values? If this is still the correct approach, maybe it would be shorter to use bool(prep.ebi_experiment_accessions.values()).
There was a problem hiding this comment.
Long day already, you are 100% right! Fixing.
There was a problem hiding this comment.
Using the .values() method wouldn't work here?
There was a problem hiding this comment.
Seems more concise, but not necessary. This is good to be merged pending @josenavas' comment.
| @@ -173,7 +173,8 @@ def study_prep_get_req(study_id, user_id): | |||
| info['start_artifact_id'] = start_artifact.id | |||
| info['youngest_artifact'] = '%s - %s' % ( | |||
| youngest_artifact.name, youngest_artifact.artifact_type) | |||
| info['ebi_experiment'] = bool(prep.ebi_experiment_accessions) | |||
| info['ebi_experiment'] = bool( | |||
There was a problem hiding this comment.
Long day already, you are 100% right! Fixing.
|
@antgonza I think the only way to test this will be to emulate a failed EBI submission adding Nones in the DB as ebi_experiment_accession numbers for one artifact. Do you think that it can be done? |
|
@josenavas, @ElDeveloper, can you guys check why this is failing ... can't find the reason! Thanks. |
|
Can't quite tell what is it that's leading to the mismatch in statuses. Could it be something to do with "reseting the database" or not resetting it? (just guessing). |
|
Thought about that and pull the latests version to check locally and it works fine. To test, I'm changing the order of the asserts in those tests ... |
|
@antgonza I think the errors here are related with the test that you added. Remember that now the tests are not independent unless you write them in that way. Probably your test is altering the data that those other tests are accessing. |
|
@josenavas the issue is that I originally only modified qiita_pet/handlers/api_proxy/tests/test_studies.py and the errors are in qiita_ware/test/test_dispatchable.py (I modified test_dispatchable.py later to see if we could get more insight into the problem) so either the DB reset is not working in test_studies.py or I'm missing something, not sure ... any ideas? |
|
|
||
| # (A) | ||
| # ignoring warnings generated when adding templates | ||
| simplefilter("ignore") |
There was a problem hiding this comment.
Already discussed with @antgonza offline but noticing here for others in the thread. The problem is that this line is ignoring the warning and this applies to all future tests
Turns out that when the EBI submission fails, we populate the ebi_experiment_accessions with Nones, which is the expected behavior.