test(handlers): tests fuer dokumentenliste und streaming-downloads - #32
Merged
Conversation
.github/workflows/test.yml nannte die Schwelle 50 fuer handlers_documents.go ausdruecklich als Provisorium: "mehrere 0%-Handler ohne eigenen Test [...] Schwelle bewusst niedrig [...] bis eigene Tests nachgezogen sind". Genau diese vier Handler bekommen jetzt Tests. Abgedeckt: - handleListDocuments, Query-Zweig: Search liefert nur IDs, der Handler hydriert jede per Get (das bewusste N+1-Muster ist damit festgehalten) - handleListDocuments, Sync-Zweig: Diff plus Folge-Cursor, der im Test wieder dekodiert und gegen id/version der Diff-Antwort geprueft wird - damit ist die Runde encodeCursor/decodeCursor als Ganzes abgedeckt - decodeCursor: leerer String liefert frischen Cursor, ungueltiges Base64 gibt 400 invalid_cursor statt 500 - handleDownloadDocumentPDF und handleDownloadPageImage: Body kommt unveraendert durch, Content-Type stimmt, Upstream-404 wird VOR dem Streaming erkannt - uploadDuplicateError.Error/GetStatus direkt handlers_documents.go steigt damit von 59.9% auf 90.5%, keine 0%-Funktion mehr in der Datei. Die Gate-Schwelle geht von 50 auf 80 - dieselbe Floor-Logik wie bei den uebrigen Eintraegen (Sicherheitsabstand unter dem gemessenen Wert), und die Datei erreicht damit die Business-Logic-Zieltabelle. Der Kommentar, der die Uebergangsschwelle begruendete, ist entsprechend ersetzt.
There was a problem hiding this comment.
Pull request overview
This PR adds missing test coverage for the document list/sync endpoint and the two streaming download handlers, and then raises the CI coverage gate for handlers_documents.go to match the project’s business-logic threshold.
Changes:
- Added handler tests for
/v1/documents(query + diff/cursor paths),decodeCursor, and both streaming download endpoints (PDF + page image). - Added direct tests for
uploadDuplicateError’serror/huma.StatusErrorbehavior. - Updated the coverage gate in CI to raise
cmd/fileee-server/handlers_documents.gofrom 50% to 80% and refreshed the rationale comment.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cmd/fileee-server/handlers_test.go |
Adds tests for document list/sync cursor behavior and streaming downloads, plus uploadDuplicateError interface methods. |
.github/workflows/test.yml |
Raises the coverage gate threshold for handlers_documents.go and updates the explanatory comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2891
to
+2908
| // TestListDocuments_InvalidCursorReturns400 prüft den Fehlerpfad von decodeCursor: ein Cursor, der | ||
| // kein Base64-URL ist, muss 400 invalid_cursor liefern — NICHT 500. Der Wert kommt vom Client und | ||
| // ist damit keine Server-Fehlfunktion. | ||
| func TestListDocuments_InvalidCursorReturns400(t *testing.T) { | ||
| _, ts := newTestServer(t, nil) | ||
|
|
||
| req := newAuthedRequest(t, http.MethodGet, ts.URL+"/v1/documents?cursor=%21kein-base64", nil) | ||
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| t.Fatalf("GET /v1/documents?cursor=...: %v", err) | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != http.StatusBadRequest { | ||
| body, _ := io.ReadAll(resp.Body) | ||
| t.Fatalf("status = %d, want 400, body=%s", resp.StatusCode, body) | ||
| } | ||
| } |
Review-Feedback aus PR #32: Der Test behauptete im Kommentar "400 invalid_cursor", pruefte aber nur den HTTP-Status. Ein Wechsel des maschinenlesbaren "code" bei weiterhin 400 waere unbemerkt geblieben - genau das ist aber die Zusage an Aufrufer, die den Fehler programmatisch auswerten. Der Test dekodiert den Body jetzt und prueft code und error.
Owner
Author
|
Zutreffend, behoben. Der Test behauptete im Kommentar „400 Der Test dekodiert den Fehler-Body jetzt und prüft beide Felder: if errBody.Code != "invalid_cursor" { ... }
if errBody.Error != "invalid cursor parameter" { ... }Der |
strausmann
added a commit
that referenced
this pull request
Aug 6, 2026
) Zieht die letzten testbaren 0%-Stellen nach, nachdem #32 dasselbe fuer handlers_documents.go getan hat. - handleListBoxes / handleGetBox: die beiden Box-Leserouten waren die einzigen Box-Handler ohne Test, Ein- und Ausheften hatten schon welche. Der List-Test laesst die Diff-Antwort bewusst ein abweichendes totalRows tragen, damit auffiele, wenn der Handler es durchreicht statt len(boxes) zu nehmen. - statusError.Error / GetStatus: beide werden sonst nur indirekt ueber Huma aufgerufen, die mapError-Tests pruefen die Felder statt der Methoden. Der Test prueft zusaetzlich die JSON-Form und haelt damit fest, dass der Status bewusst NICHT im Body steht - er ist bereits der HTTP-Status-Code. handlers_entities.go 78.0% -> 98.0%, errors.go 80.0% -> 100.0%. Die Schwellen gehen entsprechend auf 90. Ausserhalb von main.go gibt es damit keine 0%-Funktion mehr. Die dortigen fuenf (main, fatal, execFileeeServer, runInfisicalCommand, infisicalVersionCommand) kapseln os.Exit, syscall.Exec und exec.Command und bleiben bewusst ungetestet - der Kommentar im Gate haelt das jetzt fest, statt es offen zu lassen.
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
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.
Löst ein im Repo selbst dokumentiertes Provisorium ab.
.github/workflows/test.ymlbegründete die Coverage-Schwelle50fürhandlers_documents.goso:Genau diese vier Handler bekommen jetzt Tests.
Abgedeckt
handleListDocuments, beide Zweige. Der Query-Zweig prüft das bewusst in Kauf genommene N+1-Muster:Documents.Searchliefert nur Treffer-IDs, der Handler hydriert jede perDocuments.Get. Der Test hält das fest, damit ein späteres Umbauen auffällt.Der Sync-Zweig läuft über
Documents.Diffund gibt einen codierten Folge-Cursor zurück. Der Test dekodiert das Token wieder und prüft, dass es dieid/versionaus der Diff-Antwort trägt — damit ist die RundeencodeCursor→decodeCursorals Ganzes abgedeckt, nicht nur „irgendein nicht-leerer String".decodeCursor, beide Sonderfälle. Leerer String liefert einen frischen Cursor mitdocumentCursorEntityType(Voll-Sync von vorn, der Normalfall beim ersten Aufruf). Ungültiges Base64 liefert 400invalid_cursor, nicht 500 — der Wert kommt vom Client, das ist keine Server-Fehlfunktion.Beide Streaming-Downloads. Body kommt unverändert durch, Content-Type stimmt (
application/pdfbzw.image/jpeg), und ein Upstream-404 wird vor dem Streaming erkannt — der Handler darf keinen leeren 200-Stream aufmachen.uploadDuplicateError.Error()undGetStatus()direkt, da beide über dashuma.StatusError-Interface aufgerufen werden und bisher nur indirekt liefen.Coverage
handlers_documents.gogesamthandleListDocumentsdecodeCursorhandleDownloadDocumentPDFhandleDownloadPageImageError(uploadDuplicateError)Keine 0%-Funktion mehr in der Datei.
Die Gate-Schwelle geht von
50auf80— dieselbe Floor-Logik wie bei allen übrigen Einträgen (Sicherheitsabstand unter dem gemessenen Wert, kein aspirationaler Zielwert, siehe „GOLDENE REGEL" im Kommentar). Damit erreicht die Datei die Business-Logic-Zieltabelle, und die Übergangsbegründung im Kommentar ist durch die neue ersetzt.Verifikation
OKgo vet,go test ./... -race -count=1, Doc-Coverage (0 undokumentierte exportierte Symbole),gofmt -l .grüntest.yml🤖 Generated with Claude Code