[uss_qualifier] Track external file contents #260
Conversation
There was a problem hiding this comment.
LGTM overall. Just two remarks.
The following configurations have references to files but were not updated. Does this break them?
- monitoring/uss_qualifier/configurations/dev/faa/uft/uft.yaml
- monitoring/uss_qualifier/configurations/dev/non_docker/resources.yaml
- monitoring/uss_qualifier/configurations/dev/faa/uft/local_message_signing.yaml
- monitoring/uss_qualifier/configurations/dev/geoawareness_cis.yaml (unsure about this one)
Also I see one potential negative side-effect from this change, which is checking out the repo and running tests will generate in the configuration files the hashes, thus modify them. Would it make sense that all files do have their hashes set in the configuration in the repository? And if we are to change them, well we just update the hashes at the same time. (and maybe also that is also checked by make format in order to check this easily before committing in the repo :) )
| If not specified, will be populated with the hash of the external file at the time of execution.""" | ||
|
|
||
|
|
||
| def _verify_or_set_hash(file: ExternalFile, content: str) -> None: |
There was a problem hiding this comment.
nit: any reason not to put this as a member of ExternalFile?
There was a problem hiding this comment.
Nope; either would work. I'll make it a method instead.
monitoring/uss_qualifier/configurations/dev/library/resources.yaml
Outdated
Show resolved
Hide resolved
Yes, this PR would currently break any of them with the affected file references. I will fix them before merging, but we should probably think about how to reliably maintain artifacts like this that aren't exercised by CI (or maybe we remove some of those artifacts).
While the configuration in memory is mutated by adding hashes at runtime, I don't think the configuration is ever written back to the file(s) from which it was read. So, running tests should produce reports that include configurations with the hashes, but the original configuration files should remain unchanged.
We could do this, but I was hoping to basically make hash tracking an optional feature -- for users who want to maintain explicit control of when external content changes, they could handle the inconvenience of keeping hashes up to date. But, for users who just want a record of what was actually run, adding the hashes at runtime seems fine. I think our dev configurations are in the latter camp, except it's probably useful to demonstrate that explicit hashes do work, which is why I specified one of the hashes explicitly. |
These non-CI configurations should now be cleaned up: uft.yaml -> Deleted; UFT activity is complete so this configuration is no longer worth actively maintaining |
Definitely! The ones not covered by the CI were broken pretty often unfortunately.
Ah - that was not my understanding. OK then I withdraw my remark :)
Yeah agreed: since I thought that the hashes would be written back to the config file, it would have made sense to have them all. But then not here. |
* Track external file contents * Fix f3548_self_contained * Convert function to method per comments * Clean up non-CI configurations * Update hash for changed file 2110d53
Currently, there are some places in the configuration where paths to files are specified. The behavior of the test depends on the contents of these external files, yet this information is not captured in the test baseline -- that is, if the contents of one of these files changed, the behavior of the test would change but the test baseline identifier would not change. Since we want the test baseline identifier to fully describe how the test behaves, this is undesirable.
This PR adds an ExternalFile object that resources can use in their specifications when they want to use the content of an external file. This object contains a hash of that resource's content. If the hash is specified by the user (test designer), then uss_qualifier will refuse to use file content that doesn't match that hash. If the hash is not specified by the user (test designer), then uss_qualifier will populate the hash, the configuration will be augmented with this information, and the configuration saved with the report will contain this information. Since the test baseline identifier is computed as a function of the configuration (as written to the report, after this PR), these hashes will be covered by the test baseline identifier.
Some additional logging statements are added to reduce amount of time spent performing unannounced tasks (it doesn't take 6 seconds to validate the configuration).
A small bug in sequence view generation discovered during this PR's development is fixed.