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

[antithesis] Refactor compose config generation to simplify reuse #3184

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

marun
Copy link
Contributor

@marun marun commented Jul 11, 2024

Why this should be merged

Refactor generation of compose configuration for antithesis test setups to simplify reuse by subnet-evm.

How this works

Creates a new function (GenerateComposeConfig) that encapsulates generation of docker-compose.yml and associated initial db state (if testing a VM) for any antithesis test setup.

How this was tested

CI

@marun marun added the testing This primarily focuses on testing label Jul 11, 2024
@marun marun self-assigned this Jul 11, 2024
@marun marun requested a review from abi87 as a code owner July 11, 2024 02:51
@marun marun marked this pull request as draft July 11, 2024 03:54
@marun marun force-pushed the antithesis-simplify-genconfig branch from 444ab35 to bf44343 Compare July 11, 2024 04:10
@marun marun marked this pull request as ready for review July 11, 2024 04:27
Comment on lines 32 to 58
if len(targetPath) == 0 {
return errors.New("TARGET_PATH environment variable not set")
}

imageTag := os.Getenv("IMAGE_TAG")
if len(imageTag) == 0 {
return errors.New("IMAGE_TAG environment variable not set")
}

// Subnet testing requires creating an initial db state for the bootstrap node
if len(network.Subnets) > 0 {
avalancheGoPath := os.Getenv("AVALANCHEGO_PATH")
if len(avalancheGoPath) == 0 {
return errors.New("AVALANCHEGO_PATH environment variable not set")
}

pluginDir := os.Getenv("AVALANCHEGO_PLUGIN_DIR")
if len(pluginDir) == 0 {
return errors.New("AVALANCHEGO_PLUGIN_DIR environment variable not set")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than using errors.New directly in the return - can we define them as variables? There is a longstanding goal I've had to add goerr113 as a linter... But there have been a few blockers (but this would be prevented by that linter)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

I forget, why is this important?

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case it isn't super important (more of a consistency style thing). But in other locations it's important to be able to test the different error conditions more naturally (with errors.Is)

@marun marun force-pushed the antithesis-simplify-genconfig branch from 1ecdfec to 7fee75d Compare July 16, 2024 18:05
@StephenButtolph StephenButtolph added this pull request to the merge queue Jul 16, 2024
Merged via the queue into master with commit 0f6031e Jul 16, 2024
20 checks passed
@StephenButtolph StephenButtolph deleted the antithesis-simplify-genconfig branch July 16, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing This primarily focuses on testing
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants