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

FILE-13 - Add more test coverage #57

Closed
wants to merge 1 commit into from
Closed

FILE-13 - Add more test coverage #57

wants to merge 1 commit into from

Conversation

gitstart
Copy link
Contributor

What does this PR do?

  • Add more unit test coverage
  • Add more e2e test coverage

Issue reference

#50

Demo video/scrennshot:

Added unit tests for each component
image

Added E2E test for app
image

Added E2E test for WebUi mode

image

Demo E2E

https://www.loom.com/share/c1fa9d4d5cfd418badf6b4f0e93ad38d

Coverage snapshot

image
image

This code was written and reviewed by GitStart Community. Growing future engineers, one PR at a time.

@gitstart
Copy link
Contributor Author

gitstart commented Aug 24, 2023

env for webui

  • VITE_ROOT_DOMAIN
  • VITE_METRICS_ORIGIN
  • VITE_STATS_ORIGIN
  • VITE_MODE_WEBUI

env for embed mode

  • VITE_ROOT_DOMAIN
  • VITE_METRICS_ORIGIN
  • VITE_STATS_ORIGIN

@gitstart gitstart marked this pull request as ready for review August 29, 2023 09:46
@gitstart
Copy link
Contributor Author

@guanzo this PR is ready for review

Copy link

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

These new tests are great!

I'm just wondering about the stability of this project - will adding tests with this level of detail slow down the project, or speed it up?

src/pages/stats/index.tsx Show resolved Hide resolved
Copy link
Contributor

@DiegoRBaquero DiegoRBaquero left a comment

Choose a reason for hiding this comment

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

Not really familiar with FE or its testing, but, a lot of these assertions seem to be just asserting what the code is telling it to be. If we define button to have a width of X why would it be Y on the test?

@gitstart
Copy link
Contributor Author

Not really familiar with FE or its testing, but, a lot of these assertions seem to be just asserting what the code is telling it to be. If we define button to have a width of X why would it be Y on the test?

Thank you for the question. The test is of two kinds, an e2e test and a unit test The e2e test uses Puppeteer to ensure that the features work from end to end on the browser and no new additions break existing features. Same as the unit test. It ensures that it renders as expected with the appropriate parameters and that future additions would not cause breaking changes.

@GitStartHQ GitStartHQ closed this by deleting the head repository Nov 7, 2023
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.

4 participants