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

Added tester for RouterLink. #1810

Merged
merged 3 commits into from
Feb 11, 2025
Merged

Added tester for RouterLink. #1810

merged 3 commits into from
Feb 11, 2025

Conversation

joelpop
Copy link
Collaborator

@joelpop joelpop commented Jul 8, 2024

Description

Adds missing ComponentTester for RouterLink. (Attempting to use AnchorTester in its place results in an error due to Anchor and RouterLink being different Components.

Tester methods include

  • getHref
  • getPath
  • getQueryParameters
  • getRoute
  • click

Fixes #1805

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

@joelpop joelpop requested review from caalador and mcollovati July 8, 2024 21:58
@joelpop joelpop self-assigned this Jul 8, 2024
@joelpop joelpop added the UITest JUnit testing the UI label Jul 8, 2024
Comment on lines 46 to 48
var targetlessRouterLink = $routerLinkView.find(RouterLink.class)
.withText("No Target")
.single();
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest making all RouterLink components as package protected class fields so that the test can directly wrap the instance, without relying on find.
This allows to prevent false positives in tests because of potential bugs in the component locator.
IMO it also simplifies the test code

Copy link
Contributor

Choose a reason for hiding this comment

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

I would revert changes to this file that are not related to RouterLink

*/
public Component click() {
if (getRoute().isEmpty()) {
throw new IllegalStateException();
Copy link
Contributor

Choose a reason for hiding this comment

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

This exception requires an explanatory message

*
* @return navigated view
*/
public Component click() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about returning HasElement to be consistent with AnchorTester?

Copy link
Contributor

@mcollovati mcollovati Aug 5, 2024

Choose a reason for hiding this comment

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

The method should ensure that the component is usable by calling ensureComponentIsUsable().
I mean, the check is done indirectly done by getPath(), but I think it would be good to make it more explicit.

Comment on lines 81 to 83
Assertions.assertDoesNotThrow(() -> $targetView.find(Span.class)
.withText("Static Target View")
.single());
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as for RouterLink, what about having the Span as a package protected field and simply assert on its getText()?

@mcollovati mcollovati self-assigned this Jan 30, 2025
@mcollovati mcollovati force-pushed the feature/#1805-RouterLinkTester branch from dc98d56 to f2142aa Compare January 30, 2025 10:32
@mshabarov mshabarov requested review from mshabarov and removed request for caalador February 3, 2025 12:24
@mshabarov mshabarov dismissed mcollovati’s stale review February 11, 2025 09:29

Marco did changes

@mshabarov mshabarov merged commit 1768e19 into main Feb 11, 2025
2 checks passed
@mshabarov mshabarov deleted the feature/#1805-RouterLinkTester branch February 11, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UITest JUnit testing the UI
Projects
Development

Successfully merging this pull request may close these issues.

Add missing RouterLinkTester for RouterLink
3 participants