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

Fix DevTools extensions not being resolved when app's Dart entrypoint is located in integration_test directory #6644

Merged

Conversation

bartekpacia
Copy link
Member

@bartekpacia bartekpacia commented Nov 2, 2023

I'm developing a DevTools extension that's meant to be used during integration testing. That's why our app's entrypoint is integration_test/test_bundle.dart, not lib/main.dart or similar.

Unfortunately, when the app is started with integration_test/*.dart entrypoint, no DevTools extensions were loaded and we were seeing the following errors:

Serving DevTools at http://127.0.0.1:9103/.
          Hit ctrl-c to terminate the server.
[ERROR] `findExtensions` failed: package_config.json not found at: "file:///Users/julia/patrol_challenge/integration_test/test_bundle.dart/.dart_tool/package_config.json"
[ERROR] `findExtensions` failed: package_config.json not found at: "file:///Users/julia/patrol_challenge/integration_test/test_bundle.dart/.dart_tool/package_config.json"

List which issues are fixed by this PR.

I wrote about this issue on Flutter team's Discord (link).

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or there is a reason for not adding tests.

build.yaml badge

If you need help, consider asking for help on Discord.

@bartekpacia bartekpacia requested a review from a team as a code owner November 2, 2023 14:31
@bartekpacia bartekpacia requested review from CoderDake and removed request for a team November 2, 2023 14:31
bartekpacia added a commit to leancodepl/patrol that referenced this pull request Nov 2, 2023
@CoderDake CoderDake requested review from kenzieschmoll and removed request for CoderDake November 2, 2023 14:54
Copy link
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for bringing this issue to our attention. Before the next release of DevTools, I'm going to try to fix this for all cases (including integration_test) by addressing the TODO in this method:

  // TODO(kenz): for robustness, consider sending the root library uri to the
  // server and having the server look for the package folder that contains the
  // `.dart_tool` directory.

@bartekpacia
Copy link
Member Author

Thanks for quick review @kenzieschmoll. I fixed code formatting, should be good now.

Comment on lines 167 to 172
final libDirectoryRegExp = RegExp(r'\/lib\/[^\/.]*.dart');
final libDirectoryIndex = fileUri.indexOf(libDirectoryRegExp);
final integrationTestDirectoryRegExp =
RegExp(r'\/integration_test\/[^\/.]*.dart');
final integrationTestDirectoryIndex =
fileUri.indexOf(integrationTestDirectoryRegExp);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this could be a single regex, something like:

RegExp(r'\/(lib|integration_test|test|bin)\/[^\/.]*.dart');

Copy link
Member Author

Choose a reason for hiding this comment

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

let's see - I pushed the changes

@kenzieschmoll
Copy link
Member

kenzieschmoll commented Nov 3, 2023

You'll need to merge master to get this to pass the tests. Also, can you add an entry to NEXT_RELEASE_NOTES.md in the General section?

* Enabled DevTools extensions when debugging a Dart entry point that is not under `lib` (e.g. a unit
test or integration test). Thanks to @bartekpacia for this change! - [#6644](https://github.com/flutter/devtools/pull/6644)

@bartekpacia
Copy link
Member Author

Thanks - done!

@kenzieschmoll kenzieschmoll merged commit a114b48 into flutter:master Nov 3, 2023
21 checks passed
@bartekpacia bartekpacia deleted the fix/integration_test_directory branch November 3, 2023 19:14
kenzieschmoll pushed a commit that referenced this pull request Nov 7, 2023
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

3 participants