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

Migrate Angular router link tests to use RouterTestingHarness #2430

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

Conversation

jattasNI
Copy link
Contributor

@jattasNI jattasNI commented Oct 3, 2024

Pull Request

🤨 Rationale

We have several tests that verify Angular router link behavior by clicking links and inspecting router state. These tests exhibit a couple problems:

  1. currently they report warnings in the output log like: [web-server]: 404: /_karma_webpack_/page1?param1=true
  2. In Upgrade Playwright and Apache Arrow dependencies #2387 when we bring in a newer version of Chromium, these cause other tests to fail to execute and report timeouts

The root cause of these issues is that the tests are actually trying to navigate the page when the link is clicked.

👩‍💻 Implementation

In researching best practices for writing tests like this I learned that the RouterTestingModule we had been using has been deprecated and replaced with a more powerful RouterTestingHarness. This blog and video does a good job of explaining it, much better than the angular docs. The basic idea is that the harness sets up a parent component and router which host your component under test. When something tries to navigate the harness captures information about the navigation but doesn't actually navigate the page.

The fixes in this PR are to use RouterTestingHarness instead of RouterTestingModule. This has these side effects:

  1. The routes are configured with provideRouter instead of withRoutes
  2. The navigated route started to be relative to the current route, so starting from /start and clicking a link to page resulted in a URL of /start/page. The simplest fix I found for this was to change the starting page to /.
  3. Some setup code could be deleted and made sync/async instead of fakeAsync.

🧪 Testing

  1. Verified the 404 warnings are no longer printed to the console
  2. Verified the tests don't navigate the page in the newer version of Chromium
  3. TODO: verify tests fail with various changes

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@jattasNI
Copy link
Contributor Author

jattasNI commented Oct 3, 2024

@m-akinc I think I found a fix to the router test issues we talked about this week. Could you buddy the approach in this one test? If it looks good I'll apply it to the other router link tests.

@jattasNI jattasNI requested a review from m-akinc October 3, 2024 18:55
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.

1 participant