AAP-37878: move Chatbot test to vitest-browser-react #1470
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
goneri
force-pushed
the
goneri/AAP-37878-move-Chatbot-test-to-vitest-browser-react_30941
branch
9 times, most recently
from
5f3f923 to
e8a1a28
Compare
Right now, our tests are failing due to some problem with `happy-dom`, the fake webbrowser used to run the tests by `react-testing-library`. `jsdom` is an alternative but we cannot use it without adjusting our tests and facing similar issues or limitations. By moving to `vitest-browser-react`, we are now able to run the tests against a real browser like Chrome or Firefox. We still have the option to run the tests "headless" for a CI environment. `vitest-browser-react` is directly based on `vitest-browser` and allow us to stay in a standard `vite` ecosystem. It also reuses some elements from `react-testing-library`, which explains why the PR is not that large. Overall, this change reduces the differences between our tests and the production environment, and allow use to limit our exposure to the limitations of these dummy browser implementations.
goneri
force-pushed
the
goneri/AAP-37878-move-Chatbot-test-to-vitest-browser-react_30941
branch
from
e8a1a28 to
75867e4
Compare
|
|
Super star 🦸 I'll look tomorrow. |
There was a problem hiding this comment.
LGTM 👍 Just a few nit-picks.
I also had to run the following to be able to run tests locally:
npm install <!-- No sh1t Sherlock.
npx playwright install
npx playwright install-deps
The tests run successfully most of the time 🥳
There was one occasion when one failed (no big deal) but it highlighted a (possible) need for ansible_ai_connect_chatbot/src/__screenshots__/ in .gitignore.
| document.body.appendChild(debugDiv); | ||
| rootDiv = document.createElement("div"); | ||
| rootDiv.setAttribute("id", "root"); | ||
| const view = render( |
There was a problem hiding this comment.
You don't need to declare const view = ... as it is unused.
| test("Basic chatbot interaction", async () => { | ||
| mockAxios(200); | ||
| const view = await renderApp(); | ||
| const textArea = page.getByLabelText("Send a message..."); |
There was a problem hiding this comment.
You could call sendMessage("Hello") and remove L#128-131.
| return view; | ||
| } | ||
|
|
||
| async function sendMessage(message: string) { |
There was a problem hiding this comment.
The parameter message is redundant.
| }); | ||
| mockAxios(200); | ||
| const view = await renderApp(); | ||
| const textArea = page.getByLabelText("Send a message..."); |
There was a problem hiding this comment.
You could call sendMessage("Hello") and remove L#173-176.
| } | ||
| mockAxios(200, false, false, lotsOfRefDocs); | ||
| const view = await renderApp(); | ||
| const textArea = page.getByLabelText("Send a message..."); |
There was a problem hiding this comment.
You could call sendMessage("Hello") and remove L#219-222.
| expect(ghIssueLinkSpy).toEqual(1); | ||
| // // Assert the size of the resulting documents in the query parameter is 30, | ||
| // // as the max defined, instead of the 35 being present. | ||
| // const url: string | undefined = ghIssueLinkSpy.mock.calls[0][0]?.toString(); |
| test("Chat service returns 500", async () => { | ||
| mockAxios(500); | ||
| const view = await renderApp(); | ||
| const textArea = page.getByLabelText("Send a message..."); |
There was a problem hiding this comment.
You could call sendMessage("Hello") and remove L#257-260.
| test("Chat service returns a timeout error", async () => { | ||
| mockAxios(-1, true, true); | ||
| await renderApp(); | ||
| const textArea = page.getByLabelText("Send a message..."); |
There was a problem hiding this comment.
You could call sendMessage("Hello") and remove L#269-271.
| test("Chat service returns 429 Too Many Requests error", async () => { | ||
| mockAxios(429, true); | ||
| await renderApp(); | ||
| const textArea = page.getByLabelText("Send a message..."); |
There was a problem hiding this comment.
You could call sendMessage("Hello") and remove L#287-289.
| test("Chat service returns an unexpected error", async () => { | ||
| mockAxios(-1, true); | ||
| const view = await renderApp(); | ||
| const textArea = page.getByLabelText("Send a message..."); |
There was a problem hiding this comment.
You could call sendMessage("Hello") and remove L#304-306.
|
Great, thank you for rhe reviews. |
goneri
deleted the
goneri/AAP-37878-move-Chatbot-test-to-vitest-browser-react_30941
branch
Right now, our tests are failing due to some problem with
happy-dom, the fake webbrowserused to run the tests by
react-testing-library.jsdomis an alternative but we cannotuse it without adjusting our tests and facing similar issues or limitations.
By moving to
vitest-browser-react, we are now able to run the tests against a real browserlike Chrome or Firefox. We still have the option to run the tests "headless" for a CI environment.
vitest-browser-reactis directly based onvitest-browserand allow us to stay in a standardviteecosystem. It also reuses some elements fromreact-testing-library, which explains whythe PR is not that large.
Overall, this change reduces the differences between our tests and the production environment,
and allow us to limit our exposure to the limitations of these dummy browser implementations.