Bug 1846013 Allows users to fetch results from all frameworks - #9761
Closed
gopar wants to merge 17 commits into
Closed
Bug 1846013 Allows users to fetch results from all frameworks#9761gopar wants to merge 17 commits into
gopar wants to merge 17 commits into
Conversation
✅ Deploy Preview for treeherder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
gopar
commented
Aug 8, 2026
| # Build common result dictionary (contains only data both test versions use) | ||
| is_complete = base_runs_count and new_runs_count | ||
| resolved_framework = ( | ||
| framework or base_sig.get("framework_id") or new_sig.get("framework_id") |
Collaborator
Author
There was a problem hiding this comment.
Who should win here when framework is empty? base or new?
Collaborator
There was a problem hiding this comment.
The expected logic for this task is that if the framework parameter is omitted by the frontend, the system should default to displaying comparison data for all available frameworks in PerfCompare (e.g., awsy, browsertime, build_metrics, devtools). I hope this helps clarify things.
However, it’s worth noting that previous attempts to implement this triggered performance issues due to the high volume of compare results being returned. I'm not certain whether our past performance issues on this have been resolved yet. It may be too early to tackle this now, so we'll probably need to consult Sparky before taking next steps.
Collaborator
Author
There was a problem hiding this comment.
@gmierz when you get a chance can you chime in. thanks!
Collaborator
There was a problem hiding this comment.
We haven't resolved or looked into those issues. There's another issue here too which is that we're going to be mixing results from frameworks that have data which is useless to most devs (e.g. build metrics and mozharness).
I think we should land this change, but instead of having the framework be empty for all data, use all as the option so that we don't have people inadvertently querying for all the data if they omit the framework. Main reason for that is in case this causes too much load on our backend. With that, we can then test out what it looks like in PerfCompare but we shouldn't allow/expose to users there yet.
gopar
commented
Aug 8, 2026
Comment on lines
+2091
to
2100
| query_set = PerformanceSignature.objects.filter(parent_signature_id=None).prefetch_related( | ||
| "performancealert" | ||
| ) | ||
| if framework_id is not None: | ||
| query_set = query_set.filter(framework_id=framework_id) | ||
| query_set = ( | ||
| PerformanceSignature.objects.prefetch_related("performancealert") | ||
| .filter(framework_id=framework_id, parent_signature_id=None) | ||
| .values("suite", "test") | ||
| query_set.values("suite", "test") | ||
| .annotate(repositories=GroupConcat("repository_id", distinct=True)) | ||
| .annotate(platforms=GroupConcat("platform_id", distinct=True)) | ||
| .annotate(total_alerts=Count("performancealert")) |
Collaborator
Author
There was a problem hiding this comment.
So i don't see any tests related to this endpoint. Is it in use? When I tried writing a test I got some sql errors and it was because GroupConcat is a MySql feature not in postgres. is this dead code that should be removed then?
gopar
changed the title
| return Response(data=query_params.errors, status=HTTP_400_BAD_REQUEST) | ||
|
|
||
| framework_id = query_params.validated_data["framework"] | ||
| query_set = PerformanceSignature.objects.filter(parent_signature_id=None).prefetch_related( |
Collaborator
There was a problem hiding this comment.
To my knowledge, this endpoint (TestSuiteHealthViewSet) is not utilized in PerfCompare, so modifications here should not be necessary.
gopar
force-pushed
the
bug-1846013-framework-param-optional
branch
from
August 17, 2026 19:01
ec19781 to
f70ad16
Compare
Archaeopteryx
added
the
perfherder
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…ks (mozilla#9778) With a task selected, clicking a different tree in the secondary navbar opened the new repository but kept the old task's details panel (task details, summary/failures tabs) open, even though the task is not part of the new repository. Two defects composed to cause this: 1. updateRepoParams() removed the legacy selectedJob param but not the current selectedTaskRun param, so the stale selection traveled into the new repo's URL. 2. The Redux-to-Zustand migration regressed doClearSelectedJob(): the old Redux version documented and handled {} (passed by all URL-sync clear paths) as "no pinned jobs, clear unconditionally", but the Zustand version used a plain truthiness check, where {} is truthy. Every clear on the URL-sync path became a silent no-op, so the store kept the old task and the details panel stayed open. updateRepoParams() now also drops selectedTaskRun, and doClearSelectedJob() restores the number-or-object handling. All existing callers pass numbers, so their behavior is unchanged.
…st. (mozilla#9741) The Differential Revision trailer of the commit message is already part of the push data, so no additional request is needed to know it. Give the author initials a minimum width, so that the icons of the commits of a push are aligned.
…ile extension. (mozilla#9736) Follow-up to mozilla#9565, which relaxed the extension check for the artifact panel but left the failure summary regex requiring a .js.json suffix. Profiles of mochitest plain test failures are named after the test file, eg. profile_test_group_touchevents-6-2.html.json, so their "profile uploaded in" lines were rendered without a profiler link.
.rt-tbody a and a:visited have equal specificity (0,1,1), so the tie was broken by source order. treeherder-base.css is hoisted ahead of treeherder-custom-styles.css in the bundle (ui/App.jsx statically imports ./userguide/App, which is the first module to pull in base.css), which made the teal .rt-tbody rule win for visited links too. Adding .rt-tbody a:visited at specificity (0,2,1) restores purple for visited links in the intermittent failures tables and the Perfherder graph table view, while unvisited links keep #187c86. purple (#800080) on the table background is ~8.9:1, so this does not regress the contrast pass that motivated the teal rule.
…missing. (mozilla#9726) The crash viewer reads the .json file, so a crash with only a .dmp and a .json can still be opened there. Show the .extra link only when that file is present, and aggregate the size and expiry over the files we have.
We transform the name already for querying current status, but the UI links are missing that. Factor out and reuse the same logic.
* [bug-2020493] Add helper classes for easier data passing * [bug-2020493] Add helper function to reduce boiler plate * [bug-2020493] Avoid double negative in variable naming for easier reading * [bug-2020493] Move class creation to their own section * [bug-2020493] Fix conditional error * [bug-2020493] Rename variables for better readability
We were using the key `submit_time` in the hand-built datum dicts, but then during serialization, `PerformanceDatumSerializer` was looking for a `job__submit_time` key instead. That's because we supply `source="job__submit_time"` in the serializer's constructor. The mismatch failed silently rather than raising: the field is declared `required=False, default=None`, so every datum serialized with `submit_time: null`, which left the graphs view with an Invalid Date for its retrigger times. Use `job__submit_time` so that the serializer finds the field.
* Migrate UI integration tests from Puppeteer to Playwright Replace the jest-puppeteer + Polly.js integration test setup with @playwright/test: - Add playwright.config.js; the config starts (or reuses) the dev server on port 5000, matching the previous jest-puppeteer behavior. - Convert the logviewer suite to a Playwright spec, keeping the fetch-mocking approach via page.addInitScript. Selectors and labels are updated to the current toolbar UI (classic-log-toolbar-label replaced the old copy-highlight bar). - Convert the graphs view suite to a Playwright spec. The existing Polly HAR recordings are replayed with page.routeFromHAR; the performance/summary responses are served by signature because the query params have changed since the HAR was recorded. - Remove puppeteer, jest-puppeteer, jest-environment-puppeteer, the @pollyjs packages and setup-polly-jest, along with jest.integration.config.js and jest-puppeteer.config.js. - Update docs/testing.md and biome.json accordingly. * Add Playwright integration tests for the Jobs view Cover the basic Jobs view workflows end to end: rendering the push list, selecting a job and verifying the details panel opens with the job's details and the selectedTaskRun URL param, and narrowing the displayed jobs with the quick filter. API responses are served from the JSON fixtures in tests/ui/mock/ via page.route, so the tests run deterministically without a backend. * Run Playwright integration tests in Firefox by default Firefox is the preferred browser for this project. Switch the Playwright project from Chromium to Firefox and update docs/testing.md accordingly. Granting clipboard permissions is a Chromium-only API that throws on Firefox, so the logviewer copy test now only grants them when running in Chromium; Playwright's Firefox permits the clipboard write in tests without an explicit grant. * fix: regenerate pnpm-lock.yaml after rebase conflict resolution The rebase onto origin/master conflicted in pnpm-lock.yaml. Took the target branch's version as a starting point and regenerated the lockfile with `pnpm install --no-frozen-lockfile` to ensure it is consistent with package.json (playwright/puppeteer/polly changes from this branch plus master's zustand 5.0.15 bump). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Run Playwright integration tests in CircleCI Adds a javascript-integration-tests job that installs and caches the Playwright Firefox build, runs pnpm test:integration (Playwright's webServer starts the dev server itself), and uploads the JUnit results plus the HTML report/traces as artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…mozilla#9776) This allows showing per-machine data in the graphs without requiring separate requests for each job's job details. The potential costs of this change are: - Increased database query time - Increased response size The query already looks at job submit times so it's already looking at the right table, so any query time regressions should be minor. (Claude took a look in more detail and had more elaborate justifications but I didn't understand them so I'm not copying them here.) The response size grows, but gzip mostly takes care of it because there aren't a lot of different machine names. I measured a 1050-row response as an example, and it grew by 16.8% raw and 3.2% gzipped ("0.6 bytes per row").
Since the Django session was capped at AUTH_MAX_SESSION_AGE_SECONDS (mozilla#9688), any user whose renewal heartbeat stops for more than ~45 minutes (laptop asleep, browser closed overnight) loses their session. On the next page load the frontend saw the anonymous backend response and fully logged the user out -- wiping the auth0-spa-js refresh-token cache -- forcing an interactive re-login even though the refresh token was still perfectly valid. Add AuthService.recoverSession(), which uses the refresh token to silently obtain new tokens and re-establish the backend session. The page-load flow in Login now attempts this recovery before treating the user as logged out, so ordinary users stay logged in for the life of their refresh token (roughly a day or more of inactivity) while the security property of the session cap is preserved: a revoked user's refresh token fails at Auth0, recovery returns null, and they are logged out within the cap window as before.
Migrate the get_commit method in treeeherder/utils/github.py to use PyGithub object's.
…st the single job (mozilla#9684)
Collaborator
Author
|
closing PR, i mest up the git history and starting fresh |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug: API - make framework parameter optional