Skip to content

Documenting integration tests - #9051

Open
cj-radcliff wants to merge 5 commits into
flutter:mainfrom
cj-radcliff:documenting-integration-tests
Open

Documenting integration tests#9051
cj-radcliff wants to merge 5 commits into
flutter:mainfrom
cj-radcliff:documenting-integration-tests

Conversation

@cj-radcliff

Copy link
Copy Markdown
Collaborator

Thanks for your contribution! Please replace this text with:

  • a description of what this PR is changing and why
  • any relevant issues
  • a description of how to verify the change is working
  • screenshots/gif if relevant

Review the contribution guidelines below:

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
  • I've included the required information in the description above.
  • My up-to-date information is in the AUTHORS file.
  • I've updated CHANGELOG.md if appropriate.
Contribution guidelines:
  • See
    our contributor guide and
    the Flutter organization contributor guide
    for general expectations for PRs.
  • Larger or significant changes should be discussed in an issue before creating a PR.
  • Dart contributions to our repos should follow the Dart style guide and use
    dart format.
  • Java and Kotlin contributions should strive to follow Java and Kotlin best
    practices (discussion).

Add documentation for integration testing and the IntelliJ IDE Starter
framework.

- Create a new docs/integration-testing.md file to document the
  integration testing process.
- Create a new docs/intellij-ide-starter.md file to document the
  IntelliJ IDE Starter framework.
- Link to the new documentation from the README.md and
  docs/integration-testing.md files.

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

Copy link
Copy Markdown
Contributor

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 introduces documentation and automated integration tests for creating a new Flutter application using the IntelliJ IDE Starter framework. This includes a new test plan, checklist, and test files (CreateNewFlutterMobileAppTest and a commented-out WindowsDebugOutputTest), along with helper classes in NewProject.kt. Feedback on the changes highlights several critical issues: platform checkboxes are clicked blindly without checking their initial state; a syntax error exists where kotlinRadioButton.isSelected is missing parentheses; the test workflow is incomplete, ending with a hardcoded 30-second wait instead of verifying that the project opened successfully; the new test file is missing the required copyright header; there is an undocumented TODO comment; actionButtonByXpath is incorrectly used for locating labels and text fields in NewProject.kt; and the WindowsDebugOutputTest file is entirely commented out and should instead use the @Disabled annotation.

Comment on lines +77 to +81
linuxButton.click()
macOsButton.click()
webButton.click()
windowsButton.click()
Assertions.assertTrue(!linuxButton.isSelected())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

[MUST-FIX] Blindly clicking platform checkboxes can toggle them to an incorrect state depending on their default selection. Check their state using isSelected() before clicking to ensure they are robustly deselected.

Suggested change
linuxButton.click()
macOsButton.click()
webButton.click()
windowsButton.click()
Assertions.assertTrue(!linuxButton.isSelected())
if (linuxButton.isSelected()) linuxButton.click()
if (macOsButton.isSelected()) macOsButton.click()
if (webButton.isSelected()) webButton.click()
if (windowsButton.isSelected()) windowsButton.click()
Assertions.assertFalse(linuxButton.isSelected(), "Linux platform should be deselected.")

//Assertions.assertTrue(applicationTypeDropDown.getSelectedItem() == "Application")

// 9. Ensure that the android language is Kotlin
Assertions.assertTrue(kotlinRadioButton.isSelected)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

[MUST-FIX] kotlinRadioButton.isSelected is missing parentheses. It should be a method call isSelected() to match the other radio buttons and checkboxes, otherwise it may cause a compilation or runtime error.

Suggested change
Assertions.assertTrue(kotlinRadioButton.isSelected)
Assertions.assertTrue(kotlinRadioButton.isSelected())

Comment on lines +100 to +105
// 10. Click create
createButton.click()



wait(30.seconds)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

[MUST-FIX] The test is incomplete. It clicks the "Create" button and then performs a hardcoded 30-second wait before ending, completely skipping steps 11 and 12 of the test plan (verifying that the project opened and that main.dart and README.md are open in editor tabs). Hardcoded waits should also be replaced with conditional waits.

Suggested change
// 10. Click create
createButton.click()
wait(30.seconds)
// 10. Click create
createButton.click()
// TODO: Implement steps 11 and 12 to verify the project opened successfully
// and that main.dart and README.md are open in editor tabs instead of a hardcoded wait.

@@ -0,0 +1,111 @@
package io.flutter.integrationTest

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[MUST-FIX] Missing copyright header. All new source code files must have a proper copyright header as per the repository style guide.

Suggested change
package io.flutter.integrationTest
/*
* Copyright 2026 The Chromium Authors. All rights reserved.
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
package io.flutter.integrationTest
References
  1. Ensure all new source code files (.java, .kt, .dart, .sh) have a proper copyright header (e.g., Copyright 2026 The Chromium Authors). Flag any missing headers on source code files as [MUST-FIX]. (link)

Comment on lines +94 to +95
// Todo Fix this
//Assertions.assertTrue(applicationTypeDropDown.getSelectedItem() == "Application")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] Stray TODO comment without justification. The repository style guide discourages leaving undocumented TODOs or commented-out assertions without explanation.

Suggested change
// Todo Fix this
//Assertions.assertTrue(applicationTypeDropDown.getSelectedItem() == "Application")
// TODO: Implement application type verification once the driver API supports it.
// Assertions.assertTrue(applicationTypeDropDown.getSelectedItem() == "Application")
References
  1. Avoid stray TODO or FIXME comments without justification. (link)

Comment on lines +69 to +77
// Locates the "Project name" label.
val projectNameLabel = actionButtonByXpath(xQuery { byVisibleText("Project name:") })

// Locates the "Project name" input field.
//val projectNameInput = x("//div[@class='LabeledComponent' and ./div[text()='Project name:']]//div[@class='ExtendableTextField']")
val projectNameInput = actionButtonByXpath(xQuery { byVisibleText("untitled") })

// Locates the "Project location" label.
val projectLocationLabel = actionButtonByXpath(xQuery { byVisibleText("Project location:") })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] Incorrect and fragile use of actionButtonByXpath for labels and text fields. Labels are not action buttons and should be located using generic x(...) finders, while text fields should use the textField(...) helper to ensure semantic correctness and prevent runtime casting issues.

Suggested change
// Locates the "Project name" label.
val projectNameLabel = actionButtonByXpath(xQuery { byVisibleText("Project name:") })
// Locates the "Project name" input field.
//val projectNameInput = x("//div[@class='LabeledComponent' and ./div[text()='Project name:']]//div[@class='ExtendableTextField']")
val projectNameInput = actionButtonByXpath(xQuery { byVisibleText("untitled") })
// Locates the "Project location" label.
val projectLocationLabel = actionButtonByXpath(xQuery { byVisibleText("Project location:") })
// Locates the "Project name" label.
val projectNameLabel = x("//div[@text='Project name:']")
// Locates the "Project name" input field.
val projectNameInput = textField(xQuery { byType("JTextField") })
// Locates the "Project location" label.
val projectLocationLabel = x("//div[@text='Project location:']")

@@ -0,0 +1,109 @@
/*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] The entire test file is commented out. If a test is designed to fail or is not yet ready, it should be annotated with @Disabled from JUnit 5 rather than commenting out the entire file. Commenting out code prevents compilation and leads to code rot.

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