BuildReports: Add test report log - #1417
Conversation
|
this looks good to me. @bgamari i think we'll need to deploy the builder and the main hackage in sync for this... when you get time, take a look and then we can coordinate? |
|
I tried testing the migration locally, and I am getting an error with Acid.Core "too few bytes". So there must be some problem with this PR, or maybe it is exposing an existing issue... I will report back if I find out what the problem is. I think I'll try to write a unit test for this. |
|
I've done some migration testsusing the following REPL code: I still haven't discovered any issues... |
|
@gbaz Do you have any ideas for how to advance this? I'd be curious to know if you can reproduce my issue with your local instance. |
sol
left a comment
There was a problem hiding this comment.
From what I understand --test-show-details=always is a much easier (one line) fix that does not require a data migration.
sol
left a comment
There was a problem hiding this comment.
From what I understand --test-show-details=always is a much easier (one line) fix that does not require a data migration.
The test report log contains the actual output emitted by the test-suite in a given Cabal package. The test log only contains the logs of the *build* of the test.
Revert changes to existing events used in makeAcidic. They need to be kept for compatibility with old data.
20057c6 to
686358d
Compare
|
Tick the box to add this pull request to the merge queue (same as
|
|
This approach is superior since it doesn't conflate test build logs with test output logs. I have rebased this and I fixed the issue it had: It was changing the meaning of existing events in I will merge this in a few hours after pondering and verifying . |
There was a problem hiding this comment.
🟡 Changes recommended
Backups lose the new logs, and packages with multiple test suites upload only one suite’s output.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds test-suite runtime logs to build reports, separate from test build logs.
Changes:
- Captures, uploads, stores, and serves test report logs.
- Adds state migrations and supporting instances.
- Displays logs in report HTML.
File summaries
| File | Description |
|---|---|
src/Distribution/Server/Framework/ResponseContentTypes.hs |
Adds plain-text response type. |
src/Distribution/Server/Framework/MemSize.hs |
Supports five-element tuples. |
src/Distribution/Server/Framework/BlobStorage.hs |
Adds Arbitrary support for blob IDs. |
src/Distribution/Server/Features/Security/MD5.hs |
Adds Arbitrary support for MD5 digests. |
src/Distribution/Server/Features/Html.hs |
Renders test report logs. |
src/Distribution/Server/Features/BuildReports/State.hs |
Extends persisted report operations. |
src/Distribution/Server/Features/BuildReports/BuildReports.hs |
Stores logs and migrates existing state. |
src/Distribution/Server/Features/BuildReports/BuildReport.hs |
Extends upload JSON payloads. |
src/Distribution/Server/Features/BuildReports/Backup.hs |
Updates backup tuple handling. |
src/Distribution/Server/Features/BuildReports.hs |
Adds upload, query, and serving integration. |
exes/BuildClient.hs |
Captures and uploads test output. |
datafiles/templates/Html/report.html.st |
Displays test report output and raw link. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| reportToExport prefix reportId (report, mlog, _, _, _) = BackupByteString (getPath ".txt") (packUTF8 $ Report.show report) : | ||
| case mlog of Nothing -> []; Just (BuildLog blobId) -> [blobToBackup (getPath ".log") blobId] |
There was a problem hiding this comment.
This follows the existing pattern, you can see other members are already not backed up.
|
|
||
|
|
||
| testPackage :: Verbosity -> BuildOpts -> DocInfo -> IO (Maybe String, Maybe FilePath, Maybe FilePath) | ||
| testPackage :: Verbosity -> BuildOpts -> DocInfo -> IO (Maybe String, Maybe FilePath, Maybe FilePath, Maybe FilePath) |
There was a problem hiding this comment.
Incorrect. Later suites do not overwrite earlier output. All the suites reach Hackage.
The test report log contains the actual output emitted by the test-suite
in a given Cabal package.
The test log only contains the logs of the build of the test.
Fixes #1397.
Demo
These were the logs as emitted by
cabal run exe:hackage-build -- build testpkg -v:Cabal file fragment:
Main.hs (of test-suite) contents: