[ftr] migrating 'management/ingest_pipelines' API integration test to deployment-agnostic one #192063
Conversation
dmlemeshko
added
release_note:skip
There was a problem hiding this comment.
Thanks for working on this @dmlemeshko!
This approach makes sense to me. 👍 Were the ingest pipelines tests the only management tests that are identical in serverless and stateful?
Changes lgtm with two small comments/questions.
| .post(ingestPipelines.fixtures.apiBasePath) | ||
| .set('kbn-xsrf', 'xxx') | ||
| .send(pipelineRequestBody) | ||
| .expect(200); |
There was a problem hiding this comment.
Can we keep the .expect(200) part?
|
|
||
| describe('Ingest Pipelines', function () { | ||
| before(async () => { | ||
| supertestWithAdminScope = await roleScopedSupertest.getSupertestWithRoleScope('admin', { |
There was a problem hiding this comment.
Does this have the same privileges as supertest from the original test? Just wanted to make sure we don't give additional privileges that are not needed.
There was a problem hiding this comment.
supertest is inited with system_indices_superuser role, which in stateful grants absolutely "everything". In the past it hided some bugs, and generally it is not recommended to use that role for API calls testing, but create something scoped to the particular functionality.
You can see both role definitions here https://github.com/elastic/kibana/blob/main/packages/kbn-es/src/stateful_resources/roles.yml#L99-L130 , admin is a default Cloud role (Cloud Org admin) and should be able to perform all the possible operations with Kibana.
But if you think ingesting pipeline should be available with lower permissions (editor) , we should definitely change it.
There was a problem hiding this comment.
Thanks for clarifying! I think leaving it with admin role is okay given that the privileges in the original test were even higher. cc: @alisonelizabeth
I believe so only looking into |
I understand... IIRC, for some of the serverless tests we reduced the test cases and only included the most essential test cases from the corresponding stateful tests. We could use this approach to increase the test coverage in serverless for the rest of our plugins - I'll bring this up in our team sync. |
|
holding back merging until https://github.com/elastic/qaf-tests/issues/94 is resolved |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Summary
The goal is to consolidate 2 existing test files with identical logic into a single, deployment-agnostic test file:
Files to be replaced:
New test file:
The migration leverages the serverless test file as a basis for the new deployment-agnostic test.
Necessary modifications are minimal and include FTR context provider & its services to ensure compatibility across different environments.
Lastly new file has to be loaded into both the serverless and stateful config files under
x-pack/test/api_integration/deployment_agnostic/configsThis approach ensures that the test logic remains consistent while reducing redundancy and maintaince effort.