Skip to content

Fix to pass additional args from the Flutter test template (#7261)#8836

Merged
pq merged 3 commits intoflutter:mainfrom
dkandalov:fix-test-template-2
Mar 16, 2026
Merged

Fix to pass additional args from the Flutter test template (#7261)#8836
pq merged 3 commits intoflutter:mainfrom
dkandalov:fix-test-template-2

Conversation

@dkandalov
Copy link
Copy Markdown
Contributor

Fix to pass additional args from the Flutter test template (#7261)


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes an issue where additional arguments from a test run configuration template were not being preserved. The change involves explicitly copying the additionalArgs from the existing configuration to the newly created TestFields. While the fix is correct, there is some code duplication across the setupForSingleTest, setupForDartFile, and setupForDirectory methods. I've left a comment with a suggestion to refactor this for better maintainability.

Comment thread src/io/flutter/run/test/FlutterTestConfigProducer.java Outdated
@helin24
Copy link
Copy Markdown
Member

helin24 commented Mar 10, 2026

@dkandalov seems reasonable. Were you able to run this in the workbench (or build the plugin) and test it out? Can you note the different situations to test given that there are changes in a few different methods?

@helin24
Copy link
Copy Markdown
Member

helin24 commented Mar 11, 2026

I suspect this also doesn't work for run configurations. It may make sense to have a fix for run configurations as well, though this could be a separate issue.

@dkandalov
Copy link
Copy Markdown
Contributor Author

Hi @helin24.
I built the plugin via Gradle, installed and manually tested it in IntelliJ IDEA 2025.3.3.
These are the steps:

  • Modify run config "Flutter Test" template to have "--no-pub" as "Additional args".
  • Run a single test, e.g. by clicking on the gutter icon. Check that the automatically created run config has "--no-pub".
  • Run tests in a file, e.g. by using shortcut from the file explorer. Check that the automatically created run config has "--no-pub".
  • Run tests in a directory, e.g. by using shortcut from the file explorer. Check that the automatically created run config has "--no-pub".

@dkandalov
Copy link
Copy Markdown
Contributor Author

dkandalov commented Mar 13, 2026

I suspect this also doesn't work for run configurations.

Good point. I tried modifying "Flutter" template and then creating a new non-test run config for Flutter and it does have additional args from the template, so there is no issue with that 🤔

@helin24
Copy link
Copy Markdown
Member

helin24 commented Mar 13, 2026

Great, thanks for the test steps and looking into flutter run. That makes me feel more confident that having templates args in config when we run from the file is intended behavior. Can you address the one comment from gemini and also update the changelog and add yourself to the AUTHORS file?

@dkandalov
Copy link
Copy Markdown
Contributor Author

I updated AUTHORS and applied the refactoring as suggested by Gemini (the refactoring doesn't seem quite right to me but I couldn't come up something much better).

Copy link
Copy Markdown
Member

@helin24 helin24 left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments! This LGTM

@helin24 helin24 requested a review from pq March 16, 2026 16:32
Copy link
Copy Markdown
Collaborator

@pq pq left a comment

Choose a reason for hiding this comment

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

Nice. Thank you!

Comment thread src/io/flutter/run/test/FlutterTestConfigProducer.java
@pq pq merged commit b82d093 into flutter:main Mar 16, 2026
7 checks passed
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.

3 participants