Testing: replace supported_applications.txt with a field in metadata.yaml #723
Conversation
|
all the kokoro failures are unrelated, going to go ahead and merge. |
integration_test/common/common.go
Outdated
| ExpectedMetrics []*ExpectedMetric `yaml:"expected_metrics" validate:"onetrue=Representative,unique=Type,dive"` | ||
| MinimumSupportedAgentVersion MinimumSupportedAgentVersion `yaml:"minimum_supported_agent_version"` | ||
| SupportedAppVersion []string `yaml:"supported_app_version" validate:"required,unique,min=1"` | ||
| SupportedOperatingSystems string `yaml:"supported_operating_systems" validate:"required,oneof=linux windows linux_or_windows"` |
There was a problem hiding this comment.
@avilevy18 and I were discussing adding a Platform key instead, which would mark the test as Linux vs Windows. This would match the work on adding a metadata.yaml for the default agent metrics in #719.
There was a problem hiding this comment.
I'm not understanding this suggestion. Where would this key go, and how is it different from what i have besides the name? I just don't get it.
| } | ||
| allApps[app] = metadata | ||
| } | ||
| log.Printf("found %v apps", len(allApps)) |
There was a problem hiding this comment.
The old code logged the apps; this code just logs the number of apps. I wonder how this will affect debuggability.
There was a problem hiding this comment.
The impacted apps list is still logged, on line 460.
I think it's fine to just log the number of apps here, since at this point there's no realistic way for it to be wrong anyway. It has to have a valid metadata.yaml file for each app or it will return an error and never get to this point anyway.
| if supported == "linux_and_windows" { | ||
| return "" // This app supports both Linux and Windows. | ||
| } |
There was a problem hiding this comment.
Why not just omit this when both Linux and Windows are supported?
There was a problem hiding this comment.
try as I might, I can't understand this suggestion.
It is superseded by an entry in metadata.yaml. These files were meant to be deleted in #723, but it looks like i neglected to actually delete it there.
File is no longer used as of #723 but was not deleted.
This is action item #1 from this document: https://docs.google.com/document/d/1TFJ-OhIm2KmATv-sam3aLlZz8T5vMp7zsHSMxp0kVtw/edit?usp=sharing&resourcekey=0-JWBhMrd2SdUg1VYHvwuNtw
The idea is to make it impossible to forget to update
supported_applications.txt, instead requiring eachmetadata.yamlfile to contain an additional field calledsupported_operating_systems, containing eitherlinux,windows, orlinux_and_windows.This PR involved a lot of reshuffling in
third_party_apps_test.goin order to get the parsedmetadata.yamlto be accessible early on in the test. I moved themetadata.yamlparsing out ofrunSingleTestand intodetermineAllApps(which got renamed tofetchAppsAndMetadata), which runs right at the beginning of the test, before any VMs are started up.