VS-1549 Add VAT to integration tests #9085
Conversation
…dVATToIntegrationTests
scripts/variantstore/variant-annotations-table/GvsValidateVAT.wdl
Outdated
Show resolved
Hide resolved
Co-authored-by: Miguel Covarrubias <[email protected]>
…dVATToIntegrationTests
| @@ -36,6 +40,7 @@ workflow GvsQuickstartIntegration { | |||
| File full_exome_interval_list = "gs://gcp-public-data--broad-references/hg38/v0/bge_exome_calling_regions.v1.1.interval_list" | |||
| String expected_subdir = if (!chr20_X_Y_only) then "all_chrs/" else "" | |||
| File expected_output_prefix = "gs://gvs-internal-quickstart/integration/2024-10-29/" + expected_subdir | |||
| File truth_data_prefix = "gs://gvs-internal-quickstart/integration/test_data/2025-01-17/" | |||
There was a problem hiding this comment.
can you set this value in your method config during testing to avoid accidentally committing like this?
There was a problem hiding this comment.
But I think this should be test input. It's where the input vds and sites-only VCFs are. If you change this to a new path (say update our VDS) you're going to (presumably) break the test.
| @@ -85,7 +85,7 @@ workflow GvsCreateVATfromVDS { | |||
|
|
|||
| # If the vat version is undefined or v1 then the vat tables would be named like filter_vat, otherwise filter_vat_v2. | |||
| String effective_vat_version = if (defined(vat_version) && select_first([vat_version]) != "v1") then "_" + select_first([vat_version]) else "" | |||
| String vat_table_name = filter_set_name + "_vat" + effective_vat_version | |||
| String effective_vat_table_name = filter_set_name + "_vat" + effective_vat_version | |||
There was a problem hiding this comment.
not "qualified" ? whats the goal of this? maybe we should just pass the name?
There was a problem hiding this comment.
I needed vat_table_name to be an output of the wdl, and couldn't name an internal variable with the same name, so I changed it to effective_vat_table_name - using the pattern of a lot of naming nearby.
| @@ -153,7 +157,7 @@ workflow GvsValidateVat { | |||
| } | |||
|
|
|||
| # only check certain things if the callset is larger than 10,000 samples (a guess) | |||
| Boolean callset_is_small = GetNumSamplesLoaded.num_samples < 10000 | |||
| Boolean callset_is_small = select_first([is_small_callset, select_first([GetNumSamplesLoaded.num_samples, 1]) < 10000]) | |||
There was a problem hiding this comment.
maybe a comment would be helpful or am I just WDL illiterate?
There was a problem hiding this comment.
I'm adding a comment. Basically just checking if the is_small_callset flag is set, if not use the previous logic (where it's considered a small callset if there are less than 10000 samples).
Added test for VAT creation to Integration test.
It's a distinct test at the top level (that is does not follow on after the full pipeline).
Passing Integration test here
Note that this test was failing in the integration workspace after I added the part where it referenced the ancestry file which is stored in gs://gvs-internal-quickstart - I wasn't quite sure why and felt it might be an unrelated problem in our workflow, so I upped the permission of my PET account to add:
storage-admin(note that my pet account also hadstorage-object-adminandstorage-object-viewerThis was the error:
google.api_core.exceptions.Forbidden: 403 GET https://storage.googleapis.com/storage/v1/b/gvs-internal-quickstart?projection=noAcl&prettyPrint=false: [email protected] does not have storage.buckets.get access to the Google Cloud Storage bucket. Permission 'storage.buckets.get' denied on resource (or it may not exist).