Skip to content

Do not create kibana client for pipeline and static tests #1567

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 28, 2023
Merged

Do not create kibana client for pipeline and static tests #1567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
723f441
Do not create kibana client for pipeline tests
mrodm Nov 27, 2023
357f18f
Add check for required clients in test runners
mrodm Nov 27, 2023
b6e7af3
Update comment
mrodm Nov 27, 2023
5ae47c1
Use consistent casing for error messages
mrodm Nov 27, 2023
File filter

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: 8 additions & 3 deletions cmd/testrunner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/elastic/elastic-package/internal/cobraext"
"github.com/elastic/elastic-package/internal/common"
"github.com/elastic/elastic-package/internal/install"
"github.com/elastic/elastic-package/internal/kibana"
"github.com/elastic/elastic-package/internal/packages"
"github.com/elastic/elastic-package/internal/signal"
"github.com/elastic/elastic-package/internal/stack"
Expand Down Expand Up @@ -224,9 +225,13 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command
return err
}

kibanaClient, err := stack.NewKibanaClientFromProfile(profile)
if err != nil {
return fmt.Errorf("can't create Kibana client: %w", err)
var kibanaClient *kibana.Client
if testType == "system" || testType == "asset" {
// pipeline and static tests do not require a kibana client to perform their required operations
kibanaClient, err = stack.NewKibanaClientFromProfile(profile)
if err != nil {
return fmt.Errorf("can't create Kibana client: %w", err)
}
}

var results []testrunner.TestResult
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should early check in the runners that need a kibana client if it is not nil, to avoid panics, but I guess we will find these panics during tests.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just checked for the other test commands, and these are the example of outputs (static does not require neither the kibana client).

 $ elastic-package test asset -v 2023/11/27 17:22:15 DEBUG Enable verbose logging 2023/11/27 17:22:15 DEBUG Distribution built without a version tag, can't determine release chronology. Please consider using official releases at https://github.com/elastic/elastic-package/releases Run asset tests for the package 2023/11/27 17:22:15 DEBUG installing package... Error: error running package asset tests: could not complete test run: can't create the package installer: missing kibana client $ elastic-package test system -v 2023/11/27 17:23:36 DEBUG Enable verbose logging 2023/11/27 17:23:36 DEBUG Distribution built without a version tag, can't determine release chronology. Please consider using official releases at https://github.com/elastic/elastic-package/releases Run system tests for the package 2023/11/27 17:23:36 DEBUG Running system tests for data stream panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x2ee7d0c] goroutine 1 [running]: github.com/elastic/elastic-package/internal/kibana.(*Client).Version(...)	/home/mariorodriguez/Coding/work/elastic-package/internal/kibana/status.go:42 github.com/elastic/elastic-package/internal/testrunner/runners/system.(*runner).run(0xc0001bb740)	/home/mariorodriguez/Coding/work/elastic-package/internal/testrunner/runners/system/runner.go:213 +0x32c github.com/elastic/elastic-package/internal/testrunner/runners/system.(*runner).Run(0x39bdfc0?, {0xc000542720, {{0xc000374980, 0x74}, {0xc00005809b, 0x18}, {0xc0003749dc, 0x7}}, {0xc000058064, 0x4f}, ...})	/home/mariorodriguez/Coding/work/elastic-package/internal/testrunner/runners/system/runner.go:135 +0x5d github.com/elastic/elastic-package/internal/testrunner.Run({0x35c0750, 0x6}, {0xc000542720, {{0xc000374980, 0x74}, {0xc00005809b, 0x18}, {0xc0003749dc, 0x7}}, {0xc000058064, ...}, ...})	/home/mariorodriguez/Coding/work/elastic-package/internal/testrunner/testrunner.go:271 +0x95 github.com/elastic/elastic-package/cmd.testTypeCommandActionFactory.func1(0xc000625200, {0xc0002be530?, 0x0?, 0x1?})	/home/mariorodriguez/Coding/work/elastic-package/cmd/testrunner.go:239 +0xff8 github.com/spf13/cobra.(*Command).execute(0xc000625200, {0xc0002be510, 0x1, 0x1})	/home/mariorodriguez/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0xabc github.com/spf13/cobra.(*Command).ExecuteC(0xc000636000)	/home/mariorodriguez/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff github.com/spf13/cobra.(*Command).Execute(0x12cfc1a?)	/home/mariorodriguez/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039 +0x13 main.main()	/home/mariorodriguez/Coding/work/elastic-package/main.go:23 +0x65 $ elastic-package test static -v 2023/11/27 17:23:24 DEBUG Enable verbose logging 2023/11/27 17:23:24 DEBUG Distribution built without a version tag, can't determine release chronology. Please consider using official releases at https://github.com/elastic/elastic-package/releases Run static tests for the package --- Test results for package: elastic_package_registry - START --- ╭──────────────────────────┬─────────────┬───────────┬──────────────────────────┬────────┬──────────────╮ │ PACKAGE │ DATA STREAM │ TEST TYPE │ TEST NAME │ RESULT │ TIME ELAPSED │ ├──────────────────────────┼─────────────┼───────────┼──────────────────────────┼────────┼──────────────┤ │ elastic_package_registry │ metrics │ static │ Verify sample_event.json │ PASS │ 36.873029ms │ ╰──────────────────────────┴─────────────┴───────────┴──────────────────────────┴────────┴──────────────╯ --- Test results for package: elastic_package_registry - END --- Done

As for static is not required neither a kibana client, I will also remove it.
Copy link
Contributor Author

@mrodm mrodm Nov 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Added to each runner a check at the beginning of the execution to ensure that the required clients are created. That would avoid also panic errors too.
Expand Down
5 changes: 5 additions & 0 deletions internal/testrunner/runners/asset/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package asset

import (
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -64,6 +65,10 @@ func (r *runner) run() ([]testrunner.TestResult, error) {
Package: r.testFolder.Package,
})

if r.kibanaClient == nil {
return result.WithError(errors.New("missing Kibana client"))
}

testConfig, err := newConfig(r.testFolder.Path)
if err != nil {
return result.WithError(fmt.Errorf("unable to load asset loading test config file: %w", err))
Expand Down
4 changes: 4 additions & 0 deletions internal/testrunner/runners/pipeline/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ func (r *runner) run() ([]testrunner.TestResult, error) {
return nil, fmt.Errorf("listing test case definitions failed: %w", err)
}

if r.options.API == nil {
return nil, errors.New("missing Elasticsearch client")
}

dataStreamPath, found, err := packages.FindDataStreamRootForPath(r.options.TestFolder.Path)
if err != nil {
return nil, fmt.Errorf("locating data_stream root failed: %w", err)
Expand Down
7 changes: 7 additions & 0 deletions internal/testrunner/runners/system/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,13 @@ func (r *runner) run() (results []testrunner.TestResult, err error) {
logger.Debug("Running system tests for package")
}

if r.options.API == nil {
return result.WithError(errors.New("missing Elasticsearch client"))
}
if r.options.KibanaClient == nil {
return result.WithError(errors.New("missing Kibana client"))
}

stackVersion, err := r.options.KibanaClient.Version()
if err != nil {
return result.WithError(fmt.Errorf("cannot request Kibana version: %w", err))
Expand Down