Update test_get_manifest_with_files to match changes on synapse
#1473
Conversation
GiaJordan
changed the title
test_get_manifest_with_files to match changes on synapse
|
| @@ -798,4 +798,4 @@ def test_get_manifest_with_files(self, helpers, component, datasetId): | |||
| assert n_rows == 4 | |||
| elif component == "BulkRNA-seqAssay": | |||
| assert filename_in_manifest_columns | |||
| assert n_rows == 3 | |||
| assert n_rows == 4 | |||
There was a problem hiding this comment.
Nit: Does it make sense to map to specific fileview snapshot version so that this doesn't happen?
In the long term future, it would be helpful to think about tests with setup and teardown methods so that they are stable.
There was a problem hiding this comment.
The overall concern is that you want to know explicitly why a test failed. It's not ideal to have to figure out that the test failing is due to something updated on Synapse not directly related to the code.
But, that can be a separate ticket! Thanks for the great work here.
There was a problem hiding this comment.
Yeah I completely agree with what Tom said. I could create a separate ticket for this: https://sagebionetworks.jira.com/browse/FDS-2306, and this would be a good way to drive to cleaner and more organized code.
|
Also separately, is there a Jira ticket associated with this? |
There was a problem hiding this comment.
approve this as an urgent fix. More improvement will be in separate ticket: https://sagebionetworks.jira.com/browse/FDS-2306
|
Closing, #1471 will address |
Update test_get_manifest_with_files to match changes on synapse so the test doesn't fail on other branches. Other changes will be included in #1467