Closed kyleam closed 10 months ago
ooh I agree with this. As an aside, one of the types of fixes I caught was actually the case of each of the sections. In other words "Requirements" wouldnt be picked up because "requirements" was expected. I didnt inspect the actual returned item in:
spec <- mrgvalprep::read_spec_yaml(
file.path(val_dir, paste0(PKGNAME, "-stories.yaml")),
file.path(val_dir, paste0(PKGNAME, "-requirements.yaml"))
)
...but what I do know is that mrgvalidate
would report those requirements as missing. Would be awesome if we could have a check for that as well, though not sure if it belongs in this PR or is even something we want to handle via drone.
@barrettk Yeah, more involved checks like the ones you propose sound like a good thing to me. If whoever takes this on wants to skip the simpler "is the yaml valid?" check and go straight for an mrgval
-informed one (which would take care of the validity check because it of course needs to be able to parse the yaml), that works for me.
I love this idea of doing some mrgval*
checks in drone each time. They would run super quick and would give us an early heads up.
I'm not sure how much effort it would be worth putting into it, but if we could get all the way to being able to call something like mrgvalidate::find_missing()
and then do some simple checks on the result, that would probably be the best case scenario. That would likely involve some refactoring of mrgvalidate
(in addition to the .drone.yml refactoring), so maybe not worth it in the short term.
This might be a good place to start, because it would check for valid YAML and at least check some other basic formatting.
spec <- mrgvalprep::read_spec_yaml(
file.path(val_dir, paste0(PKGNAME, "-stories.yaml")),
file.path(val_dir, paste0(PKGNAME, "-requirements.yaml"))
)
I love this idea of doing some
mrgval*
checks in drone each time. They would run super quick and would give us an early heads up.I'm not sure how much effort it would be worth putting into it, but if we could get all the way to being able to call something like
mrgvalidate::find_missing()
and then do some simple checks on the result, that would probably be the best case scenario. That would likely involve some refactoring ofmrgvalidate
(in addition to the .drone.yml refactoring), so maybe not worth it in the short term.
Given that we dont care about missing tests, we could use check_input()
(newly added function, thats also called internally within create_package_docs()
), which only cares about missing links between requirements and stories (but still calls find_missing()
).
I think we could do something like:
res_df <- create_package_docs(
product_name = PKGNAME,
version = PKGVERSION,
repo_url = "git@github.com:metrumresearchgroup/bbr.git",
specs = spec,
release_notes_file = release_notes,
auto_test_dir = auto_test_dir,
output_dir = output_dir,
style_dir = style_ref_path,
write = TRUE,
cleanup_rmd = TRUE
)
if(check_input(res_df)$missing){
`drone fails`
}
check_input <- function(dd){
missing_data <- find_missing(dd) %>% suppressMessages()
missing <- purrr::map_lgl(missing_data, ~ nrow(.x) > 0)
if(any(missing[-1])){
warning("Required links between tests and/or requirements are missing. Returning missing information. Docs will not be generated")
return(
list(
missing_data = missing_data[missing],
missing = TRUE
)
)
}else{
return(
list(
missing_data = tibble(),
missing = FALSE
)
)
}
}
> res_df = create_package_docs(
+ product_name = product_name,
+ version = "vFAKE",
+ repo_url = "git@github.com:metrumresearchgroup/mrgvalidate.git",
+ specs = specs,
+ release_notes_file = file.path(TEST_INPUTS_DIR, "release_notes_sample.md"),
+ auto_test_dir = file.path(TEST_INPUTS_DIR, "validation-results-sample"),
+ output_dir = outdir_auto,
+ write = FALSE,
+ cleanup_rmd = FALSE)
$find_reqs_with_missing_tests
# A tibble: 2 × 3
RequirementId RequirementDescription TestId
<chr> <chr> <chr>
1 AMD-R001 Desktop should have a mountain on it MAN-FAKE-001
2 AMD-R001 Desktop should have the ocean on it MAN-FAKE-002
Warning message:
In check_input(test_data$dd) :
Required links between tests and/or requirements are missing. Returning missing information. Docs will not be generated
> check_input(res_df)
$missing_data
$missing_data$find_reqs_with_missing_tests
# A tibble: 2 × 3
RequirementId RequirementDescription TestId
<chr> <chr> <chr>
1 AMD-R001 Desktop should have a mountain on it MAN-FAKE-001
2 AMD-R001 Desktop should have the ocean on it MAN-FAKE-002
$missing
[1] TRUE
Warning message:
In check_input(res_df) :
Required links between tests and/or requirements are missing. Returning missing information. Docs will not be generated
With the new validation setup, this is no longer relevant.
bbr-requirements.yaml
has several syntax errors (which @barrettk has fixed in #535). To catch these before a PR is merged rather than hoping they're spotted before a release, perhaps we should add a check in Drone to make sure the yaml can be parsed.