Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload 1747/cloud agnostic e2e tests #532

Merged
merged 11 commits into from
Oct 23, 2024

Conversation

cfarmer-fearless
Copy link
Contributor

@cfarmer-fearless cfarmer-fearless commented Oct 11, 2024

Making the end to end kotlin TestNG tests more cloud agnostic and not rely on cloud services to validate results. The primary change here is rather than going in to Azure to validate results or collect information to validate against, we will use the /info endpoint of the upload api to validate results against and trust those results there.

Changes:

  • Add an environment variable to the local.properties files to name the environment being targeted.
  • Remove connecting to the Azure Blob Service from tests and instead utilize the existing uploadClient to get details from the '/info' endpoint given a UID
  • change tests to read from a "test case" with manifest details and other validations rather than just a manifest
  • Change assertions to validate from the '/info' endpoint results and make assertions more descriptive and informative when they fail.
  • Include details to assert against the target path file location and provide details in the test case about the format of this path using tokens that can be replaced with collected data
  • Simplify V1 manifest tests to pass (but these will eventually go away as V1 is completely phased out)
  • Update Processing Status tests to use a test case rather than a manifest

Copy link
Contributor Author

@cfarmer-fearless cfarmer-fearless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall! Just a few minor comments

tests/smoke/kotlin/src/test/kotlin/FileCopy.kt Outdated Show resolved Hide resolved
tests/smoke/kotlin/src/test/kotlin/ProcStat.kt Outdated Show resolved Hide resolved
data class Target(
@JsonProperty("name") val name: String,
@JsonProperty("path_template") val pathTemplate: Map<String, String>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a nested data class and assign it to the pathTemplate type instead of a generic Map<String, String>. That way, you'll be able to enforce that every path template needs a value for every environment you want to target because you'll get a runtime error if you don't provide all the values and try to deserialize.

data class PathTemplateByEnvironment(val local: String, val dev: String, val test: String, val stg: String)

Be sure to use @JsonProperty as well since the key names in the json are capital case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might need a better solution. Pulling it in like this makes the pathTemplate for each environment difficult to access when we want to just get it for the current environment unless maybe we use reflection? With the map, at least we could access it by the environment variable being set. Might need some help with figuring this one out.

tests/smoke/kotlin/src/test/kotlin/util/EnvConfig.kt Outdated Show resolved Hide resolved
@cyber-decker cyber-decker marked this pull request as ready for review October 22, 2024 18:33
@cyber-decker cyber-decker merged commit e9c8872 into main Oct 23, 2024
@cyber-decker cyber-decker deleted the UPLOAD-1747/cloud-agnostic-e2e-tests branch October 23, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants