Skip to content

Fix a unit test in DartDocumentationProvider #175

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.

Sign up for GitHub

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

Open
jwren wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix a unit test in DartDocumentationProvider #175

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ...ty/src/main/java/com/jetbrains/lang/dart/ide/documentation/DartDocumentationProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,12 @@ public PsiElement getDocumentationElementForLookupItem(PsiManager psiManager, Ob
final String dartUrl = urlResolver.getDartUrlForFile(libFile);
// "dart:html" -> "dart-html"
if (dartUrl.startsWith(DartUrlResolver.DART_PREFIX)) {
return "dart-" + dartUrl.substring(DartUrlResolver.DART_PREFIX.length());
String url = dartUrl.substring(DartUrlResolver.DART_PREFIX.length());
Copy link
Collaborator

@pq pq Jan 14, 2026

Choose a reason for hiding this comment

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

I'm trying to wrap my head around how this change would address flakiness...

Could you help me understand?

Sorry if I'm just being thick!

Copy link
Member Author

@jwren jwren Jan 14, 2026

Choose a reason for hiding this comment

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

No worries... I should have included more information in the PR comments.

Here is an example of the failure in our bots: https://github.com/flutter/dart-intellij-third-party/actions/runs/20836418630/job/59862007406?pr=198

The fix adds a normalization step that truncates the dart: URL at the first slash using substring.

  • Before: dart:core/int.dart -> dart-core/int.dart (mismatching dart-core)
  • After: dart:core/int.dart -> dart-core

This ensures that regardless of whether the resolver returns the library URI or a specific part file URI, the generated documentation key remains consistent (dart-libName), preventing hash/lookup mismatches that caused the test flakiness.

Copy link
Collaborator

@pq pq Jan 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Fascinating! So what resolver we get is non-deterministic? Is that why the test only sometimes fails?

Copy link
Collaborator

@pq pq Jan 14, 2026

Choose a reason for hiding this comment

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

Also: are we totally sure normalization is the right thing to do? I admit I don't know but my gut is that if a test is flaky, I'd expect a change there and not in the class under test -- unless the flakiness indicates a bug? Do you think that's what's going on?

int slashIndex = url.indexOf('/');
if (slashIndex > 0) {
url = url.substring(0, slashIndex);
}
return "dart-" + url;
}
}

Expand Down
Loading