Skip to content
Open
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
8 changes: 8 additions & 0 deletions cmd/testrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,8 @@ func getTestRunnerSystemCommand() *cobra.Command {
cmd.Flags().Bool(cobraext.TearDownFlagName, false, cobraext.TearDownFlagDescription)
cmd.Flags().Bool(cobraext.NoProvisionFlagName, false, cobraext.NoProvisionFlagDescription)

cmd.Flags().String(cobraext.TestDumpPrefixFlagName, "", cobraext.TestDumpPrefixFlagDescription)

cmd.MarkFlagsMutuallyExclusive(cobraext.SetupFlagName, cobraext.TearDownFlagName, cobraext.NoProvisionFlagName)
cmd.MarkFlagsRequiredTogether(cobraext.ConfigFileFlagName, cobraext.SetupFlagName)

Expand Down Expand Up @@ -476,6 +478,11 @@ func testRunnerSystemCommandAction(cmd *cobra.Command, args []string) error {
return cobraext.FlagParsingError(err, cobraext.TestCoverageFormatFlagName)
}

dumpPrefix, err := cmd.Flags().GetString(cobraext.TestDumpPrefixFlagName)
if err != nil {
return cobraext.FlagParsingError(err, cobraext.TestDumpPrefixFlagName)
}

if !slices.Contains(testrunner.CoverageFormatsList(), testCoverageFormat) {
return cobraext.FlagParsingError(fmt.Errorf("coverage format not available: %s", testCoverageFormat), cobraext.TestCoverageFormatFlagName)
}
Expand Down Expand Up @@ -585,6 +592,7 @@ func testRunnerSystemCommandAction(cmd *cobra.Command, args []string) error {
WithCoverage: testCoverage,
CoverageType: testCoverageFormat,
RepositoryRoot: repositoryRoot,
DumpPrefix: dumpPrefix,
})

logger.Debugf("Running suite...")
Expand Down
3 changes: 3 additions & 0 deletions internal/cobraext/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ const (
TestCoverageFormatFlagName = "coverage-format"
TestCoverageFormatFlagDescription = "set format for coverage reports: %s"

TestDumpPrefixFlagName = "dump-docs"
TestDumpPrefixFlagDescription = "prefix for system test document output dump file"

VariantFlagName = "variant"
VariantFlagDescription = "service variant"

Expand Down
4 changes: 4 additions & 0 deletions internal/testrunner/runners/system/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type runner struct {
generateTestResult bool
withCoverage bool
coverageType string
dumpPrefix string

configFilePath string
runSetup bool
Expand Down Expand Up @@ -78,6 +79,7 @@ type SystemTestRunnerOptions struct {
DeferCleanup time.Duration
WithCoverage bool
CoverageType string
DumpPrefix string
}

func NewSystemTestRunner(options SystemTestRunnerOptions) *runner {
Expand All @@ -100,6 +102,7 @@ func NewSystemTestRunner(options SystemTestRunnerOptions) *runner {
withCoverage: options.WithCoverage,
coverageType: options.CoverageType,
repositoryRoot: options.RepositoryRoot,
dumpPrefix: options.DumpPrefix,
}

r.resourcesManager = resources.NewManager()
Expand Down Expand Up @@ -265,6 +268,7 @@ func (r *runner) GetTests(ctx context.Context) ([]testrunner.Tester, error) {
GlobalTestConfig: r.globalTestConfig,
WithCoverage: r.withCoverage,
CoverageType: r.coverageType,
DumpPrefix: r.dumpPrefix,
})
if err != nil {
return nil, fmt.Errorf(
Expand Down
50 changes: 29 additions & 21 deletions internal/testrunner/runners/system/tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ type tester struct {
dataStreamManifest *packages.DataStreamManifest
withCoverage bool
coverageType string
dumpPrefix string

serviceStateFilePath string

Expand Down Expand Up @@ -253,6 +254,7 @@ type SystemTesterOptions struct {
GlobalTestConfig testrunner.GlobalRunnerTestConfig
WithCoverage bool
CoverageType string
DumpPrefix string

RunSetup bool
RunTearDown bool
Expand Down Expand Up @@ -705,7 +707,6 @@ func (r *tester) runTestPerVariant(ctx context.Context, stackConfig stack.Config
logger.Debugf("Using config: %q", testConfig.Name())

partial, err := r.runTest(ctx, testConfig, stackConfig, svcInfo)

tdErr := r.tearDownTest(ctx)
if err != nil {
return partial, err
Expand Down Expand Up @@ -1817,32 +1818,26 @@ func (r *tester) runTest(ctx context.Context, config *testConfig, stackConfig st
return results, nil
}

if dump, ok := os.LookupEnv(dumpScenarioDocsEnv); ok && dump != "" {
err := dumpScenarioDocs(scenario.docs)
var dumpPath string
switch {
case r.dumpPrefix != "":
dumpPath = fmt.Sprintf("%s-%s.json", r.dumpPrefix, time.Now().Format("20060102150405"))
case os.Getenv(dumpScenarioDocsEnv) != "":
dumpPath = filepath.Join(os.TempDir(), fmt.Sprintf("elastic-package-test-docs-dump-%s.json", time.Now().Format("20060102150405")))
}
var dumpErr error
if dumpPath != "" {
err := dumpScenarioDocs(scenario.docs, dumpPath)
if err != nil {
return nil, fmt.Errorf("failed to dump scenario docs: %w", err)
dumpErr = fmt.Errorf("failed to dump scenario docs: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

just for understanding, why we change the behavior, so now the error is not thrown but joined to the result of validateTestScenario. Does it make sense to run r.validateTestScenario if this fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why would it be relevant to the success of the test if some aspect of the dump fails? As an example, if the dump fails because of an fs error, do we now not care that the validation is not valid?

Copy link
Contributor

Choose a reason for hiding this comment

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

that is the question i asked in order to understand the change. before, the test was failing if dump failed, isn't it? i am just asking to understand, lacking context here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I think that behaviour was unhelpful. (see this for context)

}
}

return r.validateTestScenario(ctx, result, scenario, config)
}

func (r *tester) isTestUsingOTELCollectorInput(policyTemplateInput string) bool {
// Just supported for input packages currently
if r.pkgManifest.Type != "input" {
return false
}

if policyTemplateInput != otelCollectorInputName {
return false
}

return true
results, err := r.validateTestScenario(ctx, result, scenario, config)
return results, errors.Join(err, dumpErr)
}

func dumpScenarioDocs(docs any) error {
timestamp := time.Now().Format("20060102150405")
path := filepath.Join(os.TempDir(), fmt.Sprintf("elastic-package-test-docs-dump-%s.json", timestamp))
func dumpScenarioDocs(docs any, path string) error {
f, err := os.Create(path)
if err != nil {
return fmt.Errorf("failed to create dump file: %w", err)
Expand All @@ -1860,6 +1855,19 @@ func dumpScenarioDocs(docs any) error {
return nil
}

func (r *tester) isTestUsingOTELCollectorInput(policyTemplateInput string) bool {
// Just supported for input packages currently
if r.pkgManifest.Type != "input" {
return false
}

if policyTemplateInput != otelCollectorInputName {
return false
}

return true
}

func (r *tester) checkEnrolledAgents(ctx context.Context, agentInfo agentdeployer.AgentInfo, svcInfo servicedeployer.ServiceInfo) (*kibana.Agent, error) {
var agents []kibana.Agent

Expand Down