Skip to content

Initial read-only implementation of the try-results page #183

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Initial read-only implementation of the try-results page #183

wants to merge 6 commits into from

Conversation

athomas
Copy link
Member

@athomas athomas commented Sep 24, 2025 •
edited
Loading

  • Adds services to interact with the firestore test result data.
  • Adds a route for cl/<cl number>/<patchset number>.
  • Supports basic features for viewing CL results.
  • Enabled lints to keep imports tidy.

Known issues in try results:

  • Doesn't handle status "skipped" (currently displays it as failures).
  • Source links don't work.
  • Free-text filter doesn't work.
  • No commenting/approval feature.
  • Doesn't handle non-existing CLs/patchsets very well.

@athomas athomas force-pushed the squashed-try-results-page branch 2 times, most recently from 4e3ca97 to ed00d4f Compare September 25, 2025 14:52
@athomas athomas force-pushed the squashed-try-results-page branch 2 times, most recently from 170371b to f54cb0e Compare October 9, 2025 17:54
@athomas
Squashed commit of the following:
a95105e 
commit 50d78882baa49482b08ec8083b4c7459624d3de3
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 16 12:52:41 2025 +0200

    Format

commit c602f639127bd369ccb22f3e970ecd097cd9e3d0
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 16 12:51:34 2025 +0200

    Pre-sort try result to ensure test counts are correct

commit b196ece2b33ef6e122bf78b052c02e9eb5b8b9f7
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 16 12:40:59 2025 +0200

    Improve sorting of result changes

commit 47a094e47083cafbca47b9444ad2fad60466ff48
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 16 12:40:31 2025 +0200

    Formatting

commit d4ed80dccfbc5876549141eb6c5f83800c718385
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 16 11:19:18 2025 +0200

    Remove debug print

commit 91d39d6317d0b4c17a9f5fc594592a2f0eddf2f4
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 16 11:18:05 2025 +0200

    Move no query handling to subclass

commit 0e0e2e027f4a32578bb549d5fb440403307d7146
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 16 11:09:36 2025 +0200

    Fix ChangeInResult for new tests and refactor

commit 4bbe46f43b750076250162030591dff8c2bb0c8d
Author: Alexander Thomas <[email protected]>
Date:   Mon Sep 15 13:40:21 2025 +0200

    Delete unused changes_list.dart

commit cbc573c97fb1d90e1d4b36143167921b5934a633
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 10 06:55:25 2025 -0700

    Fix copyright headers

commit 89de18f13b255d10d6cb131496e945819637aa97
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 10 06:43:12 2025 -0700

    Upgrade dependencies

commit 7b9fbdeca87af7d2e95dcc49cf13671a6ab12ff5
Merge: f8afae5 d2efaee
Author: Alexander Thomas <[email protected]>
Date:   Fri Sep 5 18:39:44 2025 +0200

    Merge remote-tracking branch 'origin/main' into try-results-page

commit f8afae52af767bf74ad0aba7dfb25434964c4a4b
Author: Alexander Thomas <[email protected]>
Date:   Fri Sep 5 13:55:42 2025 +0200

    New implementation of try results page with current results look and feel

commit 6e03623b2f17db7fbe8ef9156eb881c5087611af
Author: Alexander Thomas <[email protected]>
Date:   Fri Sep 5 13:54:51 2025 +0200

    Add go_router dependency

commit e5c1e4a8a9e6b53646b994f4233678c38d703e74
Author: Alexander Thomas <[email protected]>
Date:   Fri Sep 5 13:52:31 2025 +0200

    Fix file header in GEMINI.md

commit 2b439b4814a1ecb6c3aa6a1a7a774945f6d3b41e
Author: Alexander Thomas <[email protected]>
Date:   Thu Sep 4 09:11:14 2025 +0200

    Add padding left

commit f1c614c351575db944c9e1effc7a6736815438dd
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 3 18:21:45 2025 +0200

    Expand rows by default and fix colors

commit d02368b58caaa54250d11c817d195bef73a9dffe
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 3 17:47:55 2025 +0200

    Add toolbar and improve header look & feel

commit 41df7da8fb2c8e33edea1c54c12dc72859267e22
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 3 17:25:47 2025 +0200

    Add link to CL and alternate row colors

commit b50e95ae79e10851e9cecc82fe90537341c538e5
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 3 12:44:39 2025 +0200

    Reformat

commit 42de59abc70013ef3548fed55438afd2ed771100
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 3 12:44:24 2025 +0200

    Use conditional import for URL strategy

commit b989fe4342ccad7729ad96099314bc8ffe984dd7
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 3 12:14:54 2025 +0200

    Update README.md and add GEMINI.md

commit 891aa19a96a00a1706900b8036be35e05775ad46
Author: Alexander Thomas <[email protected]>
Date:   Wed Sep 3 12:14:28 2025 +0200

    Add missing dependencies

commit 1274183a0937ca195c9292406255ba479712eb60
Merge: dee62f5 186e024
Author: Alexander Thomas <[email protected]>
Date:   Tue Sep 2 16:15:55 2025 +0200

    Merge branch 'main' into add-firebase-login

commit dee62f5fe53984498aea552659038e61132cd8ee
Author: Alexander Thomas <[email protected]>
Date:   Tue Aug 19 10:11:38 2025 +0200

    Add try results page

commit 8db60b5
Author: Alexander Thomas <[email protected]>
Date:   Fri Aug 15 11:28:38 2025 +0200

    Format again

commit 013e422
Author: Alexander Thomas <[email protected]>
Date:   Fri Aug 15 11:26:36 2025 +0200

    Add copyright headers

commit e682745
Author: Alexander Thomas <[email protected]>
Date:   Fri Aug 15 10:57:46 2025 +0200

    Remove extra comments

commit ea12229
Author: Alexander Thomas <[email protected]>
Date:   Fri Aug 15 10:55:02 2025 +0200

    dart format

commit 9b5b7db
Author: Alexander Thomas <[email protected]>
Date:   Fri Aug 15 10:50:01 2025 +0200

    Fix UI and tests

commit b2ed2a8
Author: Alexander Thomas <[email protected]>
Date:   Fri May 9 11:34:52 2025 +0200

    WiP to use firebase emulator

commit cf6903b
Author: Alexander Thomas <[email protected]>
Date:   Fri May 2 13:08:02 2025 +0200

    Remove some redundant comments

commit 78c9bb9
Author: Alexander Thomas <[email protected]>
Date:   Fri May 2 12:22:14 2025 +0200

    Make it work

commit 97d0a35
Author: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date:   Thu May 1 15:58:09 2025 +0000

    fix: Update Firebase config with values from console

    Updates the Firebase configuration in current_results_ui/web/index.html
    with the precise values obtained from the Firebase console, including
    the correct apiKey and appId.
@athomas athomas force-pushed the squashed-try-results-page branch from f54cb0e to a95105e Compare October 10, 2025 11:11
@athomas athomas marked this pull request as ready for review October 10, 2025 11:29
@athomas athomas requested a review from goderbauer October 10, 2025 11:29
goderbauer
goderbauer reviewed Oct 10, 2025
View reviewed changes
current_results_ui/lib/src/widgets/results_view.dart Outdated
Provider.of<QueryResultsBase>(context, listen: false).filter =
widget.filter;
}
_tabController.index = widget.initialTabIndex;
Copy link

Choose a reason for hiding this comment

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

This line surprised me a little. Since it says initialTabIndex I would have expected it's only used once in initState. Having this line could result in some surprising behavior: let's say the app first builds ResultView(initalTabIndex: 0, filter: foo). Then the user manually switches to tab 3 (initialTabIndex remains configured to 0). Now, you change the filter and rebuild a new ResultView(initalTabIndex: 0, filter: bar). This line now causes the tab to switch back to tab 0. Is that the intended behavior?

Copy link

Choose a reason for hiding this comment

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

You may not see any problems with this in your app because of the way this widget is used in this app: If I understand it correctly, changing the tab changes the URL which then has gorouter rebuild this widget with a new initialTabIndex. So, the actual tab index and initialTabIndex are always in sync. However, this is kind of brittle and a footgun if this gets ever refactored. I wonder if it would be better to just have this widget internally read from the URL what the active tab should be instead of managing this from the outside? After all, it already knows about the URL encoding since it uses that to change the tab.

goderbauer
goderbauer approved these changes Oct 13, 2025
View reviewed changes
Copy link

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@athomas @goderbauer