Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Home component with SSE mocking #824

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

cstenglein
Copy link
Collaborator

This is just a basic test at first, but it shows that it can work.

Had to fix some stuff since africa, but now it works correctly.

Will have to refine it and will take some more time, but with that my main issue with using websockets instead of sse will be fixed and #773 could be closed.

@escapedcat What do you think?

);

await waitFor(() => {
const totalBalanceValue = screen
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we just do the same, we do in the unit-tests, i.e.
expect(await screen.findByText("123,456 SAT")).toBeInTheDocument();

Adding dataTestIds is not good practice I believe. That's why we avoided these in the unit-tests as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding dataTestIds is not good practice I believe. That's why we avoided these in the unit-tests as well.

Yes, was just frustrated because I didn't get the result I wanted at first 😅

Could we just do the same, we do in the unit-tests, i.e.
expect(await screen.findByText("123,456 SAT")).toBeInTheDocument();

Agreed!
Since the messaging mock now works, I would do tests for the component (e.g. the balance test for the WalletCard) & in the Home I could check for the higher-level stuff, e.g. when does the loading spinner appear etc.

@escapedcat
Copy link
Collaborator

Looks nice! 👏
Having one question

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants