-
-
Notifications
You must be signed in to change notification settings - Fork 648
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
Regression tests for double suspense/double resource fetch #3103
Regression tests for double suspense/double resource fetch #3103
Conversation
128cb1d
to
87c8b65
Compare
This comment was marked as outdated.
This comment was marked as outdated.
I've now verified that the current test cases will fail as expected for past commits, so I am happy for this to be reviewed and merged. That said, I think d3c3b22 should be omitted from the squash, assuming if that's going to be the case (I generally try to make good commits but I can understand having testing related commits be squashed to make a more concise history). Reason is that those tests do function as a standalone set of documentation showing the difference between the effects of CSR and Hydrated rendering on subsequent resource fetches, and if this discrepancy should be corrected later the specific commit highlighting the fact may be useful. (Edit: I did a subsequent force push because I included a baseline example that I should have included in an earlier commit; correspondingly above commit id is updated.) Moreover, if the For posterity, the following are the failures against specific key historic commits related to this issue from the past: Test failures against 7831e4a (assumed to be
|
bb67d7e
to
10a0ad3
Compare
- Based on initial concepts developed for reproducing leptos-rs#2937 and others, streamlined and instrumented for e2e testing and refined for inclusion as a standalone module to be plugged into some other App.
- Instead of using examples, just feed it the table because examples will rerun the whole scenario from scratch, which isn't what we are trying to test here. - Provide a basic example with item listing to show how this will work.
- Keep all SSR calls on ticket 0 as a means to disambiguate them from CSR calls. For the mean time the focus of tests isn't on that behavior but this may be modified when suitable.
- Given the new understanding, the scenerios all being the baseline tests they are now moved into one file. - Have the checks against all calls at once for better diff output, and reword the new scenerios into more idomatic gherkin. - Streamline the steps and provide additional ones that will help with feature definitions.
- Done by providing a button directly on the top level component with the navigation footer. This will be useful for the next test.
- Specifically, under hydrated load, resources that shouldn't need refetching gets refetched, while CSR does not show this issue.
10a0ad3
to
d3c3b22
Compare
Thanks very much for doing the work to add these, and to test the tests against the past examples! I really appreciate it. |
As per advice, this has been added to the existing
suspense_tests
in examples, but given the complexity it's added as a new standalone module that exports theInstrumentedRoutes
for inclusion in some otherApp
. During this initial start I've noted some bugs with[aria-current]
for CSR (which affects the<nav>
towards the subpaths, and this is observed with the existing tests), and that theA
hrefs don't seem to format properly as there seem to be subtle differences betweenactix
andaxum
integration. I think the new test may benefit from gating behind feature flags to flip between them, and potentially theSsrMode
as documented in a comment in the new module.These tests target the issues originally reported in #2937, #2956 and #2961.
Tasks: