Skip to content

[kbn-test] improve run_check_ftr_configs_cli script#188854

Merged
dmlemeshko merged 5 commits intoelastic:mainfrom
dmlemeshko:kbn-test/improve-ftr-configs-checker
Jul 22, 2024
Merged

[kbn-test] improve run_check_ftr_configs_cli script#188854
dmlemeshko merged 5 commits intoelastic:mainfrom
dmlemeshko:kbn-test/improve-ftr-configs-checker

Conversation

@dmlemeshko
Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko commented Jul 22, 2024

Summary

Follow-up to #188825

@crespocarlos reported that some Oblt configs were missing after #187440

I was using node scripts/check_ftr_configs.js to validate I did not miss anything and decided to debug the script.

We had a pretty strict config file content validation like testRunner|testFiles, that was skipping some FTR configs like x-pack/test/apm_api_integration/basic/config.ts

I extended file content check to look for default export function and also skip test/suite or Cypress-own config files.

In the end 7 FTR configs were discovered, but only 2 are with tests. I will ask owners to confirm if it should be enabled/disabled. Script run output:

node scripts/check_ftr_configs.js
ERROR The following files look like FTR configs which are not listed in one of manifest files:
        - x-pack/plugins/observability_solution/uptime/e2e/config.ts
        - x-pack/test/functional_basic/apps/ml/config.base.ts
        - x-pack/test/functional_basic/apps/transform/config.base.ts
        - x-pack/test/security_solution_api_integration/config/ess/config.base.trial.ts
        - x-pack/test_serverless/functional/test_suites/observability/cypress/oblt_config.base.ts

      Make sure to add your new FTR config to the correct manifest file.

      Stateful tests:
      .buildkite/ftr_platform_stateful_configs.yml
      .buildkite/ftr_oblt_stateful_configs.yml
      .buildkite/ftr_security_stateful_configs.yml
      .buildkite/ftr_search_stateful_configs.yml

      Serverless tests:
      .buildkite/ftr_base_serverless_configs.yml
      .buildkite/ftr_oblt_serverless_configs.yml
      .buildkite/ftr_security_serverless_configs.yml
      .buildkite/ftr_search_serverless_configs.yml

ERROR Please add the listed paths to the correct manifest file. If it's not an FTR config, you can add it to the IGNORED_PATHS in packages/kbn-test/src/functional_test_runner/lib/config/run_check_ftr_configs_cli.ts or contact #kibana-operations

disabled:
# Base config files, only necessary to inform config finding script
- x-pack/test_serverless/functional/test_suites/security/cypress/security_config.base.ts
- x-pack/test_serverless/functional/test_suites/security/cypress/cypress.config.ts
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@MadameSheema could you confirm this config should be disabled (not run on kibana CI) ?

- x-pack/plugins/observability_solution/profiling/e2e/ftr_config.ts

#FTR configs
- x-pack/plugins/observability_solution/uptime/e2e/config.ts
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@shahzad31 could you confirm if this config should be disabled and not run on kibana CI?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it should be run, why do we want to disable it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It got lost a while ago and we didn't run it. I just fixed a script and it pointed the configs that are not in manifest file. It was one if them

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

got it, let's hope tests aren't broken since :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[kbn-test/improve-ftr-configs-checker][~/github/kibana]$ node scripts/functional_tests.js --config x-pack/plugins/observability_solution/uptime/e2e/config.ts
 warn ❗️❗️❗️
 warn ❗️❗️❗️
 warn ❗️❗️❗️
 warn    Don't forget to use `node scripts/build_kibana_platform_plugins` to build plugins you plan on testing
 warn ❗️❗️❗️
 warn ❗️❗️❗️
 warn ❗️❗️❗️
ERROR UNHANDLED ERROR
ERROR Error: No tests defined.
          at FunctionalTestRunner.runHarness (functional_test_runner.ts:233:15)
          at FunctionalTestRunner.getTestStats (functional_test_runner.ts:153:23)
          at checkForEnabledTestsInFtrConfig (run_ftr.ts:44:27)
          at run_tests.ts:72:61
          at tooling_log.ts:84:18
          at runTests (run_tests.ts:64:5)
          at description (cli.ts:24:7)
          at run.ts:73:10
          at withProcRunner (with_proc_runner.ts:29:5)
          at run (run.ts:71:5)

Somehow there are no tests defined in config. I will keep it as disabled. If there should be some tests in config, I think code owners should wok on it separately and re-enable config

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's executed via a custom script

in uptime plugin, you can do

node scripts/e2e.js

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, then it is correct to keep it under disabled: we enable only the configs that are run classical way through scripts/functional_tests

@dmlemeshko dmlemeshko added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v8.15.0 labels Jul 22, 2024
@dmlemeshko
Copy link
Copy Markdown
Contributor Author

/ci

@dmlemeshko dmlemeshko requested a review from MadameSheema July 22, 2024 15:20
@dmlemeshko dmlemeshko marked this pull request as ready for review July 22, 2024 15:20
@dmlemeshko dmlemeshko requested review from a team as code owners July 22, 2024 15:20
@dmlemeshko
Copy link
Copy Markdown
Contributor Author

/ci

Copy link
Copy Markdown
Contributor

@wayneseymour wayneseymour left a comment

Choose a reason for hiding this comment

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

Ran successfully against my local

@dmlemeshko dmlemeshko enabled auto-merge (squash) July 22, 2024 16:22
@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
@kbn/test 13 14 +1

Total ESLint disabled count

id before after diff
@kbn/test 13 14 +1

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dmlemeshko dmlemeshko merged commit 013276e into elastic:main Jul 22, 2024
dmlemeshko added a commit that referenced this pull request Jul 29, 2024
## Summary

Follow-up to #188854

On CI script is taking >1 min, while before it was taking seconds. 
Probably because 80+ files were loaded as potential FTR configs. I
adjusted regular expressions to minimize the amount of files that we
need to load to validate if it is FTR config or not.

In this CI run script took 20s (part of `Quick Checks` group)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants