Skip to content

Conversation

@eleonoramicozzi
Copy link
Contributor

@eleonoramicozzi eleonoramicozzi commented Jun 26, 2025

Summary

Closes #224911

Add test for bulk import functionality as a follow up to #223501

This test should only be run locally, not on CI.

For this reason, it has been added to a separate config for stateful and serverless, that is disabled in the ftr config files.

In order to run this test locally, you can run:

node scripts/functional_tests_server --config x-pack/test/api_integration/deployment_agnostic/configs/<stateful/serverless>/oblt.ai_assistant_local.<stateful/serverless>.config.ts

and then

node scripts/functional_test_runner --config=x-pack/test/api_integration/deployment_agnostic/configs/<stateful/serverless>/oblt.ai_assistant_local.<stateful/serverless>.config.ts

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • Unit or functional tests were updated or added to match the most common scenarios
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

@eleonoramicozzi eleonoramicozzi self-assigned this Jun 26, 2025
@eleonoramicozzi eleonoramicozzi requested a review from a team as a code owner June 26, 2025 15:51
@eleonoramicozzi eleonoramicozzi added release_note:enhancement backport:skip This PR does not require backporting Team:Obs AI Assistant Observability AI Assistant labels Jun 26, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

@eleonoramicozzi eleonoramicozzi changed the title Add manual test Add manual test for bulk import functionality Jun 26, 2025
Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

I can't recall if this.tags(['skipServerless', 'skipStateful']); skips in all CI envs but I like the idea of running this locally. We might decide to skip it locally as well if it's too slow.

@eleonoramicozzi
Copy link
Contributor Author

I can't recall if this.tags(['skipServerless', 'skipStateful']); skips in all CI envs but I like the idea of running this locally. We might decide to skip it locally as well if it's too slow.

Looking at https://docs.elastic.dev/appex-qa/ftr/knowledge-base/ftr-users/skip-tests#:~:text=skipStateful,local%20and%20MKI).
I believe that skipServerless and skipStateful will skip all CI and local. So I think it has the same effect as describe.skip basically.

We could use skipCloud instead which I think will still run it on local, but I'm not sure about what would happen with the PR CI.

@sorenlouv do you know who I could ask to make sure?

@sorenlouv
Copy link
Member

I can't recall if this.tags(['skipServerless', 'skipStateful']); skips in all CI envs but I like the idea of running this locally. We might decide to skip it locally as well if it's too slow.

Looking at https://docs.elastic.dev/appex-qa/ftr/knowledge-base/ftr-users/skip-tests#:~:text=skipStateful,local%20and%20MKI). I believe that skipServerless and skipStateful will skip all CI and local. So I think it has the same effect as describe.skip basically.

We could use skipCloud instead which I think will still run it on local, but I'm not sure about what would happen with the PR CI.

@sorenlouv do you know who I could ask to make sure?

Is it correct, that what you want is to skip it everywhere (CI, MKI, ECH) but run locally? I think @pheyos will know if that's possible (he'll probably ask why we want this uncommon behaviour :D)

@pheyos
Copy link
Member

pheyos commented Jun 27, 2025

For tests that are not meant to run in any of our regular CI runs (local or cloud), we usually create a separate config file and list that under the disabled section in the config manifests (here for oblt stateful, here for oblt serverless - actually similar to the Cypress entries that in there already). Leaving corresponding comments in the config and/or test file will help to avoid misunderstandings. That way, you could specifically trigger these particular tests without grep or something and the structure will be very clear.

Technically, you could also use a combination of skip tags to achieve the same, but I would only recommend this approach if you think the complete CI skip is a temporary thing and you plan to change these labels soon.

@pheyos
Copy link
Member

pheyos commented Jun 27, 2025

why we want this uncommon behaviour

I would indeed be curious to learn about your use case here. However, it sounds like you actually might run these tests locally and only skip cloud? FWIW, we have a number of test configs that are triggered only from separate pipeline or in certain workflows, so it's not a super rare thing to do.

@eleonoramicozzi eleonoramicozzi requested review from a team as code owners June 27, 2025 16:37
@eleonoramicozzi
Copy link
Contributor Author

@pheyos I've updated the PR following your recommendations, thank you!

@csr csr self-requested a review June 30, 2025 08:03
@csr csr removed their request for review June 30, 2025 09:11
@eleonoramicozzi eleonoramicozzi requested a review from pheyos June 30, 2025 13:32
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #9 / ESQL execution logic API @ess @serverless ES|QL rule type shard failures should handle shard failures and include warning in logs for query that is not aggregating

Metrics [docs]

✅ unchanged

History

cc @eleonoramicozzi

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

Setup for local-only / non-CI tests LGTM

@eleonoramicozzi eleonoramicozzi merged commit a52e9ff into elastic:main Jul 1, 2025
10 checks passed
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
## Summary

Closes elastic#224911

Add test for bulk import functionality as a follow up to
elastic#223501

This test should only be run locally, not on CI. 

For this reason, it has been added to a separate config for stateful and
serverless, that is disabled in the ftr config files.

In order to run this test locally, you can run:
```
node scripts/functional_tests_server --config x-pack/test/api_integration/deployment_agnostic/configs/<stateful/serverless>/oblt.ai_assistant_local.<stateful/serverless>.config.ts
```
and then
```
node scripts/functional_test_runner --config=x-pack/test/api_integration/deployment_agnostic/configs/<stateful/serverless>/oblt.ai_assistant_local.<stateful/serverless>.config.ts
```
### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
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:enhancement Team:Obs AI Assistant Observability AI Assistant v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Obs AI Assistant] Add test for knowledge base bulk import functionality

6 participants