Added test to check "no tests are required" summary (e.g. checked by Bodhi).
Signed-off-by: Lukas Holecek hluk@email.cz
Optional: if not sub_policies
if not sub_policies
With None value, I'm indicating that the gating.yaml is missing. Not sure if the result of parsing the file can be empty policy list.
None
Can we document that if the file exists but it's empty, it'll not qualify as missing-gating-yaml ?
missing-gating-yaml
@lholecek, if a RemotePolicy is required but it's gating.yml is missing, will that requirement be skipped with this change? I don't think it will, just double checking :smile:
gating.yml
@lholecek, if a RemotePolicy is required but it's gating.yml is missing, will that requirement be skipped with this change? I don't think it will, just double checking ๐
missing-gating-yaml will be listed in satisfied_requirements (if RemotePolicy is required but gating.yml is missing).
satisfied_requirements
RemotePolicy
Could that be an issue?
Hmm, currently, the code is confusing. The new MissingGatingYaml answer has is_required = False but it's still listed in satisfied_requirements. (The is_required is needed to have summary with no tests are required even if there are only missing gating.yaml files.)
MissingGatingYaml
is_required = False
is_required
no tests are required
Maybe list those answers in a new response field instead?
I think it's currently clear from the doc (added emphasis):
If gating.yaml file is missing (i.e. not present in dist-git repo of the tested package in the required revision), "missing-gating-yaml" will appear in satisfied requirements.
@lholecek, if a RemotePolicy is required but it's gating.yml is missing, will that requirement be skipped with this change? I don't think it will, just double checking ๐ missing-gating-yaml will be listed in satisfied_requirements (if RemotePolicy is required but gating.yml is missing). Could that be an issue? Hmm, currently, the code is confusing. The new MissingGatingYaml answer has is_required = False but it's still listed in satisfied_requirements. (The is_required is needed to have summary with no tests are required even if there are only missing gating.yaml files.) Maybe list those answers in a new response field instead? That sounds reasonable to me. @gnaponie, thoughts?
missing-gating-yaml will be listed in satisfied_requirements (if RemotePolicy is required but gating.yml is missing). Could that be an issue? Hmm, currently, the code is confusing. The new MissingGatingYaml answer has is_required = False but it's still listed in satisfied_requirements. (The is_required is needed to have summary with no tests are required even if there are only missing gating.yaml files.) Maybe list those answers in a new response field instead? That sounds reasonable to me. @gnaponie, thoughts?
That is a bit different from how I pictured it... I was thinking more: you get the same response you will get now, but with an additional key like: "missing-gating-yaml": True
Or maybe something like: "missing-gating-yaml": [list of matching policies ids that have a remote rule that has no gating.yaml]
Or maybe instead of that list we might put the list of the NVRs for which we failed to retrieve the gating.yaml.
I don't think it is correct to put "missing-gating-yaml" as "type". The gating.yaml can be missing, but this doesn't mean that the policies are not satisfied. We agreed in the past that we shouldn't block the other policies if the remote policies cannot be found, and I think this is still correct. I would expect complains if we change this behavior.
I think this PR shouldn't change the behavior. Just add a field in the response.
I'm closing this because we want to do this differently but I don't like adding missing-gating-yaml to the response either - it could be overly complicated (rules will need to modify fields in response?) and solves very rare cases (remote rule on-boarding).
Pull-Request has been closed by lholecek
Added test to check "no tests are required" summary (e.g. checked by
Bodhi).
Signed-off-by: Lukas Holecek hluk@email.cz