Add new policy_api_format setting in system tests to force the use of an specific policy API format#1103
Conversation
… an specific policy API format
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds an optional data_stream test configuration property Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@test/packages/bad_policy_api_format/data_stream/foo/elasticsearch/ingest_pipeline/default.yml`:
- Around line 3-10: The ingest pipeline fixture is invalid because the processor
lacks a tag and the on_failure handler does not set event.kind or the required
failure context fields; update the processors entry (the "- set" that sets
field: sample_field) to include a tag (e.g., tag: sample_set) and replace the
on_failure block (the "- set" that sets field: error.message value: '{{
_ingest.on_failure_message }}' ) with handlers that set event.kind:
pipeline_error and set error.message and other fields to include '{{
_ingest.on_failure_processor_type }}', '{{ _ingest.on_failure_processor_tag }}',
and '{{ _ingest.pipeline }}' per the required contract so the pipeline is valid
and only triggers the intended policy_api_format error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f9a8f56-57ee-4e6e-8f59-fc892bad9151
⛔ Files ignored due to path filters (2)
test/packages/bad_policy_api_format/img/sample-logo.svgis excluded by!**/*.svgtest/packages/bad_policy_api_format/img/sample-screenshot.pngis excluded by!**/*.png
📒 Files selected for processing (13)
code/go/pkg/validator/validator_test.gospec/changelog.ymlspec/integration/data_stream/_dev/test/system/config.spec.ymltest/packages/bad_policy_api_format/LICENSE.txttest/packages/bad_policy_api_format/changelog.ymltest/packages/bad_policy_api_format/data_stream/foo/_dev/test/system/test-default-config.ymltest/packages/bad_policy_api_format/data_stream/foo/agent/stream/stream.yml.hbstest/packages/bad_policy_api_format/data_stream/foo/elasticsearch/ingest_pipeline/default.ymltest/packages/bad_policy_api_format/data_stream/foo/fields/base-fields.ymltest/packages/bad_policy_api_format/data_stream/foo/manifest.ymltest/packages/bad_policy_api_format/docs/README.mdtest/packages/bad_policy_api_format/manifest.ymltest/packages/good_v3/data_stream/foo/_dev/test/system/test-default-config.yml
test/packages/bad_policy_api_format/data_stream/foo/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
| policy_api_format: | ||
| description: |- | ||
| Tests can create policies using the Fleet APIs with different formats. The "legacy" format requires to | ||
| send variables with hints about their type, and defaults are not managed automatically. | ||
| The newer, "simplified" format has better format support for variables and don't allow to include hints, | ||
| it also uses defaults automatically, and requires explicit enablement or disablement of inputs and streams. | ||
| In most cases tests should not set this value. The main use case is to set it to "legacy" for cases that | ||
| cannot be fully supported with the newer "simplified" format. | ||
| type: string | ||
| enum: | ||
| - legacy | ||
| - simplified |
There was a problem hiding this comment.
This PR adds this new setting just for system tests, would it be helpful to add this same setting to the configuration of policy tests?
It looks to me that they could have the same issues that you have found in system tests.
There was a problem hiding this comment.
I haven't found any package having problems with this in policy tests, so I decided to add it only to system tests. But it is true that we have much less policy than system tests. Let me try to reproduce it...
There was a problem hiding this comment.
Confirmed with a test package, I will add it, just in case it is needed when adding policy tests to some package.
💚 Build Succeeded
History
cc @jsoriano |
elastic/elastic-package#3307 adds a new setting that allows to select per test the format used in policy API requests.
This change includes the definition for this setting.