[Obs AI Assistant] Use Mocha & Synthtrace for evaluation framework #173993
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
dgieselaar
added
release_note:skip
jbudz
approved these changes
Dec 27, 2023
CoenWarmer
reviewed
Dec 28, 2023
| @@ -39,6 +39,8 @@ interface ChatClient { | |||
| {}: { conversationId?: string; messages: InnerMessage[] }, | |||
| criteria: string[] | |||
| ) => Promise<EvaluationResult>; | |||
| getResults: () => EvaluationResult[]; | |||
| onResult: (cb: (result: EvaluationResult) => void) => () => void; | |||
There was a problem hiding this comment.
Could you add these methods to x-pack/plugins/observability_ai_assistant/public/types.ts and the mock of ChatClient in x-pack/plugins/observability_ai_assistant/public/mock.tsx?
I'm surprised the TS check did not catch this. We should probably fix this
There was a problem hiding this comment.
Different chat client, this is only for the tests
There was a problem hiding this comment.
Oh wait, this is a different ChatClient in /scripts. Should they have the same methods?
There was a problem hiding this comment.
It's a simplified client so it's easy to use for Almu & Marco as well, so I don't think we should have parity with the other client as a goal necessarily
|
Code looks good. Just one question: why use mocha instead of jest? |
|
@CoenWarmer Jest comes with a lot more things than Mocha. E.g. I don't need mocking, or watch mode, or fancy terminal output. Mocha is the simple option. |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Dec 28, 2023
Two changes here: 1. Use Mocha instead of custom test runner. This allows us to use Mocha's much more extensive API to run the tests: skipping tests, before and after hooks, grep etc. 2. Make Synthtrace available for running tests. One note: - I've added a rule to automatically inject a type reference for Mocha, via `@kbn/ambient-ftr-types`. The reason is Mocha is not typed by default (ie, the types are not available in the global space), because it conflicts with Jest. The rule enforces that `@kbn/ambient-ftr-types` is imported so things like `before()` can be used. (cherry picked from commit 5359ebe)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Dec 28, 2023
# Backport This will backport the following commits from `main` to `8.12`: - [[Obs AI Assistant] Use Mocha & Synthtrace (#173993)](#173993) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Dario Gieselaar","email":"dario.gieselaar@elastic.co"},"sourceCommit":{"committedDate":"2023-12-28T15:13:24Z","message":"[Obs AI Assistant] Use Mocha & Synthtrace (#173993)\n\nTwo changes here:\r\n\r\n1. Use Mocha instead of custom test runner. This allows us to use\r\nMocha's much more extensive API to run the tests: skipping tests, before\r\nand after hooks, grep etc.\r\n2. Make Synthtrace available for running tests.\r\n\r\nOne note:\r\n- I've added a rule to automatically inject a type reference for Mocha,\r\nvia `@kbn/ambient-ftr-types`. The reason is Mocha is not typed by\r\ndefault (ie, the types are not available in the global space), because\r\nit conflicts with Jest. The rule enforces that `@kbn/ambient-ftr-types`\r\nis imported so things like `before()` can be used.","sha":"5359ebe9c8838bd5b3fb347d1dbd556fcd88d566","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v8.12.0","v8.12.1","v8.13.0"],"title":"[Obs AI Assistant] Use Mocha & Synthtrace for evaluation framework","number":173993,"url":"https://github.com/elastic/kibana/pull/173993","mergeCommit":{"message":"[Obs AI Assistant] Use Mocha & Synthtrace (#173993)\n\nTwo changes here:\r\n\r\n1. Use Mocha instead of custom test runner. This allows us to use\r\nMocha's much more extensive API to run the tests: skipping tests, before\r\nand after hooks, grep etc.\r\n2. Make Synthtrace available for running tests.\r\n\r\nOne note:\r\n- I've added a rule to automatically inject a type reference for Mocha,\r\nvia `@kbn/ambient-ftr-types`. The reason is Mocha is not typed by\r\ndefault (ie, the types are not available in the global space), because\r\nit conflicts with Jest. The rule enforces that `@kbn/ambient-ftr-types`\r\nis imported so things like `before()` can be used.","sha":"5359ebe9c8838bd5b3fb347d1dbd556fcd88d566"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/173993","number":173993,"mergeCommit":{"message":"[Obs AI Assistant] Use Mocha & Synthtrace (#173993)\n\nTwo changes here:\r\n\r\n1. Use Mocha instead of custom test runner. This allows us to use\r\nMocha's much more extensive API to run the tests: skipping tests, before\r\nand after hooks, grep etc.\r\n2. Make Synthtrace available for running tests.\r\n\r\nOne note:\r\n- I've added a rule to automatically inject a type reference for Mocha,\r\nvia `@kbn/ambient-ftr-types`. The reason is Mocha is not typed by\r\ndefault (ie, the types are not available in the global space), because\r\nit conflicts with Jest. The rule enforces that `@kbn/ambient-ftr-types`\r\nis imported so things like `before()` can be used.","sha":"5359ebe9c8838bd5b3fb347d1dbd556fcd88d566"}}]}] BACKPORT--> Co-authored-by: Dario Gieselaar <dario.gieselaar@elastic.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two changes here:
One note:
@kbn/ambient-ftr-types. The reason is Mocha is not typed by default (ie, the types are not available in the global space), because it conflicts with Jest. The rule enforces that@kbn/ambient-ftr-typesis imported so things likebefore()can be used.