Skip to content

Update package name used in test folders - Asset runner #2517

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

Merged
merged 1 commit into from
Apr 7, 2025

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Apr 4, 2025

This PR updates the package strings set for the TestFolders in the Asset Runner/Tester to be the same as in the other tests (policy, system, static and pipeline).

This mainly affects those packages whose folder name is different of the name specified in their manfiest.yml.

This package name is used in the xUnit reports and it can be seen the difference in those values for the asset tests classname attribute:

  • Before
    • Asset
    <testsuites>
      <testsuite name="asset" tests="29">
      <!-- test suite for asset tests -->
        <testcase name="asset test: dashboard apache-Logs-Apache-Dashboard is loaded" classname="apache." time="1.979e-06"/>
        <testcase name="asset test: dashboard apache-Metrics-Apache-HTTPD-server-status is loaded" classname="apache." time="5.8e-07"/>
        ...
      </testsuite>
    </testsuites>
    • Static
    <testsuites>
      <testsuite name="static" tests="3">
      <!-- test suite for static tests -->
        <testcase name="static test: Verify sample_event.json" classname="apache_basic_license.access" time="0.115197493"/>
        <testcase name="static test: Verify sample_event.json" classname="apache_basic_license.error" time="0.100131448"/>
        <testcase name="static test: Verify sample_event.json" classname="apache_basic_license.status" time="0.079474684"/>
      </testsuite>
    </testsuites>
  • After
    • Asset
      <testsuites>
        <testsuite name="pipeline" tests="18">
          <!--test suite for pipeline tests-->
          <testcase name="pipeline test: (ingest pipeline warnings test-access-basic.log)" classname="apache_basic_license.access" time="0.298683383"></testcase>
          <testcase name="pipeline test: (ingest pipeline warnings test-access-darwin.log)" classname="apache_basic_license.access" time="0.329030781"></testcase>
          ...
        </testsuite>
      </testsuites>
    • Static
      <testsuites>
        <testsuite name="static" tests="3">
          <!--test suite for static tests-->
          <testcase name="static test: Verify sample_event.json" classname="apache_basic_license.access" time="0.081731747"></testcase>
          <testcase name="static test: Verify sample_event.json" classname="apache_basic_license.error" time="0.059626114"></testcase>
          <testcase name="static test: Verify sample_event.json" classname="apache_basic_license.status" time="0.052784676"></testcase>
        </testsuite>
      </testsuites>

@mrodm mrodm self-assigned this Apr 4, 2025
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @mrodm

@mrodm mrodm marked this pull request as ready for review April 4, 2025 15:20
@mrodm mrodm requested a review from a team April 4, 2025 15:20
testers := []testrunner.Tester{
NewAssetTester(AssetTesterOptions{
PackageRootPath: r.packageRootPath,
KibanaClient: r.kibanaClient,
TestFolder: testrunner.TestFolder{Package: r.packageRootPath},
TestFolder: testrunner.TestFolder{Package: pkg},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same change that it is applied in other test runners or testers:

for _, dataStream := range dataStreams {
folders = append(folders, TestFolder{
Path: filepath.Join(dataStreamsPath, dataStream, "_dev", "test", string(testType)),
Package: filepath.Base(packageRootPath),
DataStream: dataStream,
})
}

_, pkg := filepath.Split(packageRootPath)
for idx, p := range paths {
dataStream := ExtractDataStreamFromPath(p, packageRootPath)
folder := TestFolder{
Path: p,
Package: pkg,
DataStream: dataStream,
}
folders[idx] = folder
}

@@ -146,7 +146,7 @@ func (r *tester) run(ctx context.Context) ([]testrunner.TestResult, error) {
for _, e := range expectedAssets {
rc := testrunner.NewResultComposer(testrunner.TestResult{
Name: fmt.Sprintf("%s %s is loaded", e.Type, e.ID),
Package: installedPackage.Name,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

installedPackage.Name is the name set in the manifest.yml , but this could be different from the package folder name. As it happens, for instance, with the sql_input package in the integrations repository (link):

  • Folder name: sql_input
  • Package name in the manifest: sql

@jsoriano
Copy link
Member

jsoriano commented Apr 7, 2025

Ideally we should use the name in the manifest as package name, but I see the inconveniences.

Maybe at some point we can have both values, the name in the manifest, and the path information.

@mrodm mrodm merged commit cd25a81 into elastic:main Apr 7, 2025
3 checks passed
@mrodm mrodm deleted the update_asset_package_name_tests branch April 7, 2025 10:06
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