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

fix: print test name instead of config name #1961

Merged
merged 2 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions tests/e2e/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func parseArguments() (err error) {

type testStepsWithConfig struct {
config TestConfigType
steps []Step
steps StepChoice
}

func getTestCases(selectedPredefinedTests, selectedTestFiles TestSet, providerVersions,
Expand Down Expand Up @@ -316,13 +316,12 @@ func getTestCases(selectedPredefinedTests, selectedTestFiles TestSet, providerVe
log.Fatalf("Step choice '%s' not found.\nsee usage info:\n%s", tc, getTestCaseUsageString())
}

testSteps := stepChoices[tc].steps
if testConfig == "" {
testConfig = stepChoices[tc].testConfig
}
tests = append(tests, testStepsWithConfig{
config: testConfig,
steps: testSteps,
steps: stepChoices[tc],
Copy link
Contributor

Choose a reason for hiding this comment

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

Direct assignment of stepChoices[tc] to steps simplifies the code and ensures correct usage. Consider adding more detailed logging for better traceability.

- log.Fatalf("Step choice '%s' not found.\nsee usage info:\n%s", tc, getTestCaseUsageString())
+ log.Fatalf("Step choice '%s' not found. Available choices are: %v.\nSee usage info:\n%s", tc, keys(stepChoices), getTestCaseUsageString())
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
steps: stepChoices[tc],
steps: stepChoices[tc],
log.Fatalf("Step choice '%s' not found. Available choices are: %v.\nSee usage info:\n%s", tc, keys(stepChoices), getTestCaseUsageString())

},
)
}
Expand Down Expand Up @@ -351,7 +350,11 @@ func getTestCases(selectedPredefinedTests, selectedTestFiles TestSet, providerVe

tests = append(tests, testStepsWithConfig{
config: testConfig,
steps: testCase,
steps: StepChoice{
name: testFileName,
steps: testCase,
description: fmt.Sprintf("Steps from file %s", testFileName),
},
})
}

Expand Down
20 changes: 12 additions & 8 deletions tests/e2e/test_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const (
// It sets up the test environment and the test driver to run the tests
type TestRunner struct {
config TestConfig
steps []Step
stepChoice StepChoice
testDriver TestCaseDriver
target ExecutionTarget
verbose bool
Expand Down Expand Up @@ -86,7 +86,7 @@ func (tr *TestRunner) Run() error {
}

tr.testDriver = GetTestCaseDriver(tr.config)
err = tr.testDriver.Run(tr.steps, tr.target, tr.verbose)
err = tr.testDriver.Run(tr.stepChoice.steps, tr.target, tr.verbose)
Copy link
Contributor

Choose a reason for hiding this comment

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

The update to use tr.stepChoice.steps is correct. Consider enhancing error messages for better debugging.

- return fmt.Errorf("test run '%s' failed: %v", tr.config.name, err)
+ return fmt.Errorf("test run '%s' using step choice '%s' failed: %v", tr.config.name, tr.stepChoice.name, err)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
err = tr.testDriver.Run(tr.stepChoice.steps, tr.target, tr.verbose)
err = tr.testDriver.Run(tr.stepChoice.steps, tr.target, tr.verbose)
if err != nil {
return fmt.Errorf("test run '%s' using step choice '%s' failed: %v", tr.config.name, tr.stepChoice.name, err)
}

if err != nil {
tr.result.Failed()
// not tearing down environment for troubleshooting reasons on container
Expand Down Expand Up @@ -118,13 +118,13 @@ func (tr *TestRunner) Setup(testCfg TestConfig) error {
return nil
}

func CreateTestRunner(config TestConfig, steps []Step, target ExecutionTarget, verbose bool) TestRunner {
func CreateTestRunner(config TestConfig, stepChoice StepChoice, target ExecutionTarget, verbose bool) TestRunner {
return TestRunner{
target: target,
steps: steps,
config: config,
verbose: verbose,
result: TestResult{Status: TEST_STATUS_NOTRUN},
target: target,
stepChoice: stepChoice,
config: config,
verbose: verbose,
result: TestResult{Status: TEST_STATUS_NOTRUN},
}
}

Expand All @@ -133,8 +133,10 @@ func (tr *TestRunner) Info() string {
return fmt.Sprintf(`
------------------------------------------
Test name : %s
Config: %s
Target: %s
------------------------------------------`,
tr.stepChoice.name,
tr.config.name,
tr.target.Info(),
)
Expand All @@ -144,12 +146,14 @@ func (tr *TestRunner) Report() string {
return fmt.Sprintf(`
------------------------------------------
Test name : %s
Config: %s
Target: %s
- Status: %s
- Result: %s
- Duration: %s
- StartTime: %s
------------------------------------------`,
tr.stepChoice.name,
tr.config.name,
tr.target.Info(),
tr.result.Status,
Expand Down
Loading