GlobalPathogenAnalysisService / gpas-cli

The CLI client for GPAS SC2
Other
5 stars 2 forks source link

gpas-cli 0.6.0 - phrasing was changed for some of the error messages #100

Closed KuzminaAnna closed 1 year ago

KuzminaAnna commented 1 year ago

7 EC auto tests were broken as a result of the recent changes in the error messages phrasing. Here's the full list of changes we've discovered so far:

  1. Preconditions: csv with 1 record, where instrument_platform = "" expected error message: "instrument_platform cannot be empty" actual error message: "could not infer upload CSV schema. For Nanopore samples, column 'instrument_platform' must be 'Nanopore', and either column 'fastq' or column 'bam' must be valid paths. For Illumina samples, column 'instrument_platform' must be 'Illumina' and either columns 'bam' or 'fastq1' and 'fastq2' must be valid paths."

  2. Preconditions: csv file without headers expected: "Failed to parse CSV (Index sample_name invalid)" actual: "failed to parse upload CSV (Index sample_name invalid)"

  3. Preconditions: csv with 3 records, where Country 1 = "United States Of America", Country 2 = "FRA", Country 3 = "UK" expected: "United States Of America is not a valid ISO-3166-1 alpha-3 country code" actual: "United States Of America is not a valid ISO 3166-1 alpha-3 country code"

expected: "UK is not a valid ISO-3166-1 alpha-3 country code" actual: "invalid region (ISO 3166-2 subdivision) for specified country"

expected: "One or more regions are not valid ISO-3166-2 subdivisions for the specified country" actual: "invalid region (ISO 3166-2 subdivision) for specified country"

  1. Preconditions: csv with 3 records, where Region 1 = "TX", Region 2 = "Finistère", Region 3 = "!oxfords" expected: "TX is not a valid ISO-3166-2 subdivision name" actual: "TX is not a valid ISO 3166-2 subdivision name"

expected: "!oxfords is not a valid ISO-3166-2 subdivision name" actual: "invalid region (ISO 3166-2 subdivision) for specified country"

expected: "One or more regions are not valid ISO-3166-2 subdivisions for the specified country" actual: "invalid region (ISO 3166-2 subdivision) for specified country"

  1. Preconditions: csv with 1 record, where Region = " " expected: " is not a valid ISO-3166-2 subdivision name" actual: " is not a valid ISO 3166-2 subdivision name" at CommandLineMacTest.emptySpaceRegionTest(CommandLineMacTest.java:514)

expected: "One or more regions are not valid ISO-3166-2 subdivisions for the specified country" actual: "invalid region (ISO 3166-2 subdivision) for specified country"

  1. Preconditions: csv with 1 record, where collection_date is empty. expected: "collection_date must be in format YYYY-MM-DD between 2019-01-01 and 2022-11-15" actual: "collection_date must be in format YYYY-MM-DD between 2019-01-01 and 2022-11-21"

  2. Preconditions: csv with 3 records, where Control 1 = "", Control 2 = "None", Control 3 = "" expected: "None in the control field is not valid; field must be either empty or contain the one of the keywords negative, positive" actual: "None in the control field is not valid; field must be either empty or contain the one of the keywords positive, negative"

bede commented 1 year ago

Thanks @KuzminaAnna

  1. I'm intrigued by this discrepency as Git blame shows that the function for schema selection has not changed in > 3 months. However this error message is delivering intended behaviour. Because the poor legacy decision to create a different upload CSV schema for paired, unpaired and bam uploads, it is necessary to infer which schema has been provided prior to performing schema-based validation. An empty instrument field prevents inference of the schema and thus fails as a prevalidation check. This error message is accurate and clear if somewhat unwieldy.
  2. This change was made to increase consistency with other messages
  3. This is more technically accurate than the old message
  4. This is more technically accurate than the old message
  5. This is more technically accurate than the old message
  6. This is expected behaviour, because today's date is something that changes! If you like I can make it say today's date instead of a changing date literal.
  7. This is because the allowed values are are stored as an unordered set whose display order is stochastic. I will sort these in order to normalise display order
bede commented 1 year ago
  1. Contains a typo that I will fix in 0.8.x
bede commented 1 year ago

This has been discussed further and addressed in latest release