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

Update README to include testing example and helper #324

Closed
wants to merge 3 commits into from

Conversation

antgonzales
Copy link

No description provided.

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (30362e2) 100.00% compared to head (d24b483) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #324   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          228       228           
  Branches        56        56           
=========================================
  Hits           228       228           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@antgonzales
Copy link
Author

@molefrog bump when you have a moment to review and thank you!

Copy link
Owner

@molefrog molefrog left a comment

Choose a reason for hiding this comment

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

Hi @antgonzales. Sorry that it took me so long to look at your PR.
The README was missing the "Testing" section, so thank you for your help.

I'm a bit worried though that the example provided here is about testing React components with React testing library, rather than testing wouter apps. In my opinion, the example is a bit too specific, there are 4 unit tests included, that mostly check that the right route is rendered.

There is another discussion happening in a separate thread, where we discuss bringing back testing helpers that no longer present in the latest version. These helpers provided support for mocking the current location without modifying window.location (primarily, because calling pushState changes the history stack and since tests usually run in the same process this might result in unwanted side-effects). I see two options here: 1) wait for that discussion to be resolved and document those testing helpers 2) modify the current PR to simplify the example and focus more on how location can be mocked in tests.

Copy link
Owner

@molefrog molefrog left a comment

Choose a reason for hiding this comment

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

See #113 and #330

@antgonzales
Copy link
Author

Thanks @molefrog for taking a look! I'm confused on the overall direction of your response.

I'm a bit worried though that the example provided here is about testing React components with React testing library, rather than testing wouter apps.

Wouter is primarily a React and Preact routing tool and self-proclaimed alternative to React Router. If I'm going to use wouter, it's because I'm writing a React application, not a wouter application. It also looks like the wouter project makes heavy use of RTL in the tests, is the concern about Preact?

In my opinion, the example is a bit too specific, there are 4 unit tests included, that mostly check that the right route is rendered.

I settled on these examples using the examples from the existing README including:

  • 404 routes
  • A resource route i.e. /orders
  • A route by parameter id i.e. /orders/1
  • A redirected route

What else should I be testing if I'm a consumer of wouter?

I appreciate your feedback and time, let me know how I can better structure the tests to give consumers like me some examples and inspiration for putting together a test suite.

@molefrog
Copy link
Owner

Hi @antgonzales! I will try to provide an example to illustrate my point here. When you are writing unit tests, it is preferable to have each test as pure as possible, meaning that it's better to keep it only dedicated to the thing you are testing, not the dependencies that this feature is using. It's also better to have isolation, so that each case covers only one component of the system and not the whole system (otherwise, this becomes an integration test).

This is why there are function mocks and test adapters (for example, when testing the rendering of a collection on a page, you can replace your fetching library with a mock one that returns data fixtures). In wouter, there are ways to mock the current location without relying on pushState. This is especially useful when the test is running in the environment that doesn't implement History API, for example Node without js-dom. I must admit, that this is poorly documented and the hook that we used for that staticLocation is getting deprecated. We are working on a new hook called memoryLocation that has some improvement and I hope we will release it very soon!

I think the example you have provided is definitely useful, it is just it is out of scope of the library and it mostly tests the routing features and not the features of the app itself. To provide an analogy, let's say you have a React app, and you have a unit test that checks that the component was updated when setState had been called. Obviously, this is really useful because this is functionality that had been already thoroughly tested by the library. In this example, React is a dependency so we assume that it is working correctly.

Here is what I suggest:

  1. We can keep the paragraph that says that wouter applications can be tested just like any other React apps. We can also say that we can recommend @testing-library/react for this and provide a link to their guide that covers testing react apps.
  2. We can also mention that there is a way to mock current location by providing a static location hook to the top-level Router component.

@molefrog
Copy link
Owner

molefrog commented Mar 3, 2024

I have completely rewrote the README in v3 and included a chapter on testing using the latest API in 1f6964f

The v2 branch is locked, so I have to close this issue. Thank you for your participation @antgonzales

@molefrog molefrog closed this Mar 3, 2024
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.

None yet

2 participants