Skip to content

Jest flakiness testing#176005

Closed
jbudz wants to merge 12 commits intoelastic:mainfrom
jbudz:test-jest-flaky
Closed

Jest flakiness testing#176005
jbudz wants to merge 12 commits intoelastic:mainfrom
jbudz:test-jest-flaky

Conversation

@jbudz
Copy link
Copy Markdown
Contributor

@jbudz jbudz commented Jan 31, 2024

Not for merging

jbudz added 2 commits January 31, 2024 11:54
@jbudz
Revert "[Security Solution] [Elastic AI Assistant] Adds internal `Get…
b201bf6 
… Evaluate` API and migrates `Post Evaluate` API to OAS (elastic#175338)"

This reverts commit 38f0a7a.
@jbudz
skip tests
e04f487
@jbudz
Copy link
Copy Markdown
Contributor Author

jbudz commented Jan 31, 2024

buildkite test this

@jbudz
Copy link
Copy Markdown
Contributor Author

jbudz commented Jan 31, 2024

buildkite test this

@jbudz
Copy link
Copy Markdown
Contributor Author

jbudz commented Jan 31, 2024

buildkite test this

@jbudz
Copy link
Copy Markdown
Contributor Author

jbudz commented Jan 31, 2024

buildkite test this

@jbudz
Reapply "[Security Solution] [Elastic AI Assistant] Adds internal `Ge…
b39cc87 
…t Evaluate` API and migrates `Post Evaluate` API to OAS (elastic#175338)"

This reverts commit b201bf6.
@jbudz
Copy link
Copy Markdown
Contributor Author

jbudz commented Jan 31, 2024

buildkite test this

@jbudz
Copy link
Copy Markdown
Contributor Author

jbudz commented Jan 31, 2024

buildkite test this

@jbudz jbudz mentioned this pull request Jan 31, 2024
Merged
2 tasks
@spong
Copy link
Copy Markdown
Member

spong commented Jan 31, 2024

buildkite test this

@spong
Copy link
Copy Markdown
Member

spong commented Jan 31, 2024 •
edited
Loading

Notes:

Bringing back changes from original PR (#175338) via 6eacc9b to reintroduce the issue and to start narrowing down to root cause.

It's interesting that the errors are coming out of x-pack/plugins/security_solution/public/common/jest.config.js. since the offending PR only changed assistant code. This leads me to believe some of the structural refactoring is what is affecting a separate security solution test. At this moment, I have not found a debugging method to surface the offending test that is resulting in an update in an asynchronous callback, so will audit security solution assistant tests.

Current suspicions:

  • 🔴 Separation of evaluation api tests from api.tsx -> evaluate.test.tsx changed execution timing and surfaced pre-existing issue.
  • 🔴 Refactoring of types in elastic_assistant plugin's post_evaluate.test.ts to use types imported from @kbn/elastic-assistant-common. There's suspicion of imports causing the issue as discussed here.
  • 🟢 Refactoring of kbn-elastic-assistant package api tests from http.fetch -> http.get/post.
  • Audit security solution assistant tests
    • To be tested...

Links:

Summary:

Debugging via the above steps, it was identified that the refactoring of the http.fetch calls within the kbn-elastic-assistant package was resulting in sporadic jest failures within the security solution plugin as http.get was mocked differently within the security solution's MockAssistantProvider. TBD as to why it was sporadic and not a consistent failure, but suspicion is that an existing test suite did not rely on the mocked value from http.get to pass, and the mis-match resulted from a timeout that surfaced after the test completed (thus causing the async error logged).

Upon further debugging locally, I was able to consistently get an output failure for the x-pack/plugins/security_solution/public/common/components/top_n/index.test.tsx suite when running all security solution jest tests. Upon further inspection, it looks like the last test in the suite would be problematic as it's doing a forEach of tests over an array of 4 items, mounting the above test providers (which contains the mock). Incidentally, the number of The document global was defined's logs were 4 per run as well. Skipping just this test ends up resulting in no error or failures, so I think we've isolated it to this test. I've included a fix for this by unmount()'ing the wrapper after each test

Re-submission of original PR, which includes fix for above test here: #176025

@spong
Copy link
Copy Markdown
Member

spong commented Jan 31, 2024

buildkite test this

@spong
Copy link
Copy Markdown
Member

spong commented Jan 31, 2024

buildkite test this

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Jan 31, 2024

💔 Build Failed

Failed CI Steps

History

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

@spong spong mentioned this pull request Jan 31, 2024
Merged
2 tasks
spong added a commit that referenced this pull request Feb 1, 2024
@spong @kibanamachine
[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate…
859e440 
… API and migrates Post Evaluate API to OAS (#176025)

> [!IMPORTANT]
> This PR is a reintroduction of
#175338, which was
[reverted](#175338 (comment))
due to sporadic jest failures within the `security_solution` plugin.
Root cause was identified and detailed in
#176005 (comment).


## Summary

In #174317 we added support for
OpenAPI codegen, this PR builds on that functionality by migrating the
`Post Evaluate` route `/internal/elastic_assistant/evaluate` to be
backed by an OAS, and adds a basic `Get Evaluate` route for rounding out
the enhancements outlined in
elastic/security-team#8167 (to be in a
subsequent PR).

Changes include:
* Migration of `Post Evaluate` route to OAS
* Migration of `Post Evaluate` route to use versioned router
* Extracted `evaluate` API calls from
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/api.tsx` to
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/evaluate/evaluate.tsx`
  * Co-located relevant `use_perform_evaluation` hook  
* Adds `Get Evaluate` route, and corresponding `use_evaluation_data`
hook. Currently only returns `agentExecutors` to be selected for
evaluation.
* API versioning constants added to
`x-pack/packages/kbn-elastic-assistant-common/impl/schemas/index.ts`
* Adds new `buildRouteValidationWithZod` function to
`x-pack/plugins/elastic_assistant/server/schemas/common.ts` for
validating routes against OAS generated zod schemas.




### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@jbudz jbudz closed this Feb 1, 2024
fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 7, 2024
@spong @kibanamachine @fkanout
[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate…
c0c2605 
… API and migrates Post Evaluate API to OAS (elastic#176025)

> [!IMPORTANT]
> This PR is a reintroduction of
elastic#175338, which was
[reverted](elastic#175338 (comment))
due to sporadic jest failures within the `security_solution` plugin.
Root cause was identified and detailed in
elastic#176005 (comment).


## Summary

In elastic#174317 we added support for
OpenAPI codegen, this PR builds on that functionality by migrating the
`Post Evaluate` route `/internal/elastic_assistant/evaluate` to be
backed by an OAS, and adds a basic `Get Evaluate` route for rounding out
the enhancements outlined in
elastic/security-team#8167 (to be in a
subsequent PR).

Changes include:
* Migration of `Post Evaluate` route to OAS
* Migration of `Post Evaluate` route to use versioned router
* Extracted `evaluate` API calls from
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/api.tsx` to
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/evaluate/evaluate.tsx`
  * Co-located relevant `use_perform_evaluation` hook  
* Adds `Get Evaluate` route, and corresponding `use_evaluation_data`
hook. Currently only returns `agentExecutors` to be selected for
evaluation.
* API versioning constants added to
`x-pack/packages/kbn-elastic-assistant-common/impl/schemas/index.ts`
* Adds new `buildRouteValidationWithZod` function to
`x-pack/plugins/elastic_assistant/server/schemas/common.ts` for
validating routes against OAS generated zod schemas.




### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
@spong @kibanamachine @CoenWarmer
[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate…
495ee87 
… API and migrates Post Evaluate API to OAS (elastic#176025)

> [!IMPORTANT]
> This PR is a reintroduction of
elastic#175338, which was
[reverted](elastic#175338 (comment))
due to sporadic jest failures within the `security_solution` plugin.
Root cause was identified and detailed in
elastic#176005 (comment).


## Summary

In elastic#174317 we added support for
OpenAPI codegen, this PR builds on that functionality by migrating the
`Post Evaluate` route `/internal/elastic_assistant/evaluate` to be
backed by an OAS, and adds a basic `Get Evaluate` route for rounding out
the enhancements outlined in
elastic/security-team#8167 (to be in a
subsequent PR).

Changes include:
* Migration of `Post Evaluate` route to OAS
* Migration of `Post Evaluate` route to use versioned router
* Extracted `evaluate` API calls from
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/api.tsx` to
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/evaluate/evaluate.tsx`
  * Co-located relevant `use_perform_evaluation` hook  
* Adds `Get Evaluate` route, and corresponding `use_evaluation_data`
hook. Currently only returns `agentExecutors` to be selected for
evaluation.
* API versioning constants added to
`x-pack/packages/kbn-elastic-assistant-common/impl/schemas/index.ts`
* Adds new `buildRouteValidationWithZod` function to
`x-pack/plugins/elastic_assistant/server/schemas/common.ts` for
validating routes against OAS generated zod schemas.




### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
@spong @kibanamachine @fkanout
[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate…
abe8a6e 
… API and migrates Post Evaluate API to OAS (elastic#176025)

> [!IMPORTANT]
> This PR is a reintroduction of
elastic#175338, which was
[reverted](elastic#175338 (comment))
due to sporadic jest failures within the `security_solution` plugin.
Root cause was identified and detailed in
elastic#176005 (comment).


## Summary

In elastic#174317 we added support for
OpenAPI codegen, this PR builds on that functionality by migrating the
`Post Evaluate` route `/internal/elastic_assistant/evaluate` to be
backed by an OAS, and adds a basic `Get Evaluate` route for rounding out
the enhancements outlined in
elastic/security-team#8167 (to be in a
subsequent PR).

Changes include:
* Migration of `Post Evaluate` route to OAS
* Migration of `Post Evaluate` route to use versioned router
* Extracted `evaluate` API calls from
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/api.tsx` to
`x-pack/packages/kbn-elastic-assistant/impl/assistant/api/evaluate/evaluate.tsx`
  * Co-located relevant `use_perform_evaluation` hook  
* Adds `Get Evaluate` route, and corresponding `use_evaluation_data`
hook. Currently only returns `agentExecutors` to be selected for
evaluation.
* API versioning constants added to
`x-pack/packages/kbn-elastic-assistant-common/impl/schemas/index.ts`
* Adds new `buildRouteValidationWithZod` function to
`x-pack/plugins/elastic_assistant/server/schemas/common.ts` for
validating routes against OAS generated zod schemas.




### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jbudz @spong @kibana-ci