docs: correct the statements the release review falsified

Each of these described behaviour the code does not have:

- The nil-item note on Submit and the comment on its test claimed a rejected submission ends the
  run, while the feed keeps accepting items; the test now pins that follow-up submissions are still
  accepted and complete.
- The buildOption comment claimed every repeated option is last-wins, although the job and device
  options accumulate.
- The usesCache comment claimed stored hashes are always read, while HashRead never looks one up;
  the function decides whether a run needs a cache at all.
- TESTING.md claimed the non-linear pre-allocation on /dev/full reports ErrTargetIO. Measured on
  Linux, fallocate there returns ENODEV, which matches none of the classified errors (ENOSPC,
  EROFS, EIO), so the target carries the device's own error.
- TESTING.md's "every signature cache test" filter missed TestRunHoldsTargetCacheUntilTheHashIsComplete
  and TestManagedSignatureIsNotOrdinaryCopiedXattr.
- The Windows vet diagnostic that make check does not run is documented where the cross-platform
  commands live, with the reason it is accepted instead of worked around.
This commit is contained in:
Samuel Cui committed 2026-09-19 16:58:32 +08:00
1 parent 04aedf325b
commit 730fb0efb4
4 files changed
+34 -11

No files matched your search

+13 -3
View File
@@ -206,8 +206,11 @@ go test -race -run '^(TestRunMapsDeviceFullToTargetNoSpace|TestDeviceFullWriteFa
```
These three select no test on another platform: the file that defines them is Linux-only. Note
that `/dev/full` refuses `fallocate` with `ENODEV`, so the non-linear pre-allocation there reports
`ErrTargetIO`; the `ErrTargetNoSpace` identity for that path comes from the filled-volume test.
that `/dev/full` refuses `fallocate` with `ENODEV`, which is none of the errors a target operation
classifies (`ENOSPC`, `EROFS`, `EIO`): the non-linear pre-allocation there fails that target with
the device's own `ENODEV`, not with `ErrTargetIO` and not with `ErrTargetNoSpace`. The
`ErrTargetNoSpace` identity for that path comes from the filled-volume test, which produces a real
`ENOSPC`.
Run hash policy tests, which pin the reuse, read, and refresh matrix, and the no-op policy of
a file system without the managed signature attribute:
@@ -221,7 +224,7 @@ cache entry never travels through a reopened path, and the publication-order tes
item publishes its entry before its result:
```sh
go test -run '^(TestCachedSignatureCodec|TestSignatureCache|TestRunSignature|TestRunHashPolicy|TestRunRefresh|TestRunTransfer|TestOverwriteInvalidates|TestRunCorruptSignature|TestRunReadsTheStoredHash|TestRunWrites|TestRunPublishes)' .
go test -run '^(TestCachedSignatureCodec|TestSignatureCache|TestRunSignature|TestRunHashPolicy|TestRunRefresh|TestRunTransfer|TestOverwriteInvalidates|TestRunCorruptSignature|TestRunReadsTheStoredHash|TestRunWrites|TestRunPublishes|TestRunHoldsTargetCache|TestManagedSignature)' .
```
Run `acp-rewrite` tests, which pin one rewrite through its scratch file, the state and report
@@ -263,6 +266,13 @@ Compile the Windows-specific root package tests without running them:
GOOS=windows GOARCH=amd64 go test -c -o /tmp/acp-windows.test .
```
`make check` builds for Windows but vets only the host platform. `GOOS=windows GOARCH=amd64 go
vet ./...` reports `possible misuse of unsafe.Pointer` in `mmap/mmap_windows.go`, on the
`unsafe.Slice` call that turns the address `syscall.MapViewOfFile` returned into the mapped slice.
That conversion is the supported way to read a mapped address, vet's `unsafeptr` check cannot know
that a syscall result is a valid pointer, and the only formulation it accepts is the deprecated
`reflect.SliceHeader` pattern, so the diagnostic is accepted instead of worked around.
## Change checklist
Before submitting a change:
+4 -2
View File
@@ -102,7 +102,8 @@ func NewStream(ctx context.Context, onResults func([]Result) error, opts ...Opti
return newStream(ctx, onResults, opt)
}
// buildOption applies every option in order, so a repeated option is last-wins.
// buildOption applies every option in order, so a repeated value option is last-wins while the
// options that accumulate build on what came before them.
func buildOption(opts ...Option) (*option, error) {
opt := newOption()
for _, o := range opts {
@@ -158,7 +159,8 @@ func newStream(ctx context.Context, onResults func([]Result) error, opt *option)
// results callback returned an error, or after a fatal pipeline failure.
//
// A single submitter is required, and Items must be submitted before Close. A nil item is a
// submission error and ends the run.
// submission error that does not end the feed: it is recorded as the run's error, so Wait reports
// it, and the items submitted after it are still accepted.
func (c *StreamCopyer) Submit(items ...Item) error {
if err := c.feedStop(); err != nil {
// An exhausted linear target stays an item outcome: the items that reached it carry
+13 -5
View File
@@ -641,13 +641,14 @@ func TestStoppedLinearTargetRefusesSubmission(t *testing.T) {
}
}
// TestSubmitRejectsANilItem pins the feed's input validation: a nil item is a submission error
// that ends the run, and the items accepted before it keep their outcome.
// TestSubmitRejectsANilItem pins the feed's input validation: a nil item is a submission error that
// is recorded as the run error without ending the feed, and every accepted item keeps its outcome.
func TestSubmitRejectsANilItem(t *testing.T) {
root := t.TempDir()
input := writeSourceFile(t, root, "source.txt", []byte("fixture"))
item := newFixtureItem(input, filepath.Join(root, "target.txt"))
fixture := newStreamFixture(item)
after := newFixtureItem(input, filepath.Join(root, "target-after.txt"))
fixture := newStreamFixture(item, after)
stream, err := NewStream(context.Background(), fixture.onResults)
if err != nil {
@@ -659,14 +660,21 @@ func TestSubmitRejectsANilItem(t *testing.T) {
if err := stream.Submit(nil); err == nil {
t.Fatal("Submit(nil) error = nil, want a rejected item")
}
// A rejected submission is that submission's error, not a stopping reason: the run still
// accepts what the caller submits next.
if err := stream.Submit(after); err != nil {
t.Fatalf("Submit() after a rejected item error = %v, want the item accepted", err)
}
if err := stream.Close(); err != nil {
t.Fatalf("Close() error = %v", err)
}
if err := stream.Wait(); err == nil {
t.Fatal("Wait() error = nil, want the rejected item")
}
if _, err := item.terminal(t); err != nil {
t.Fatalf("item failed: %v", err)
for _, submitted := range []*fixtureItem{item, after} {
if _, err := submitted.terminal(t); err != nil {
t.Fatalf("item failed: %v", err)
}
}
}
+4 -1
View File
@@ -79,7 +79,10 @@ func (p HashPolicy) producesHash() bool {
return p != HashOff
}
// usesCache reports whether stored hashes are read at all.
// usesCache reports whether the policy touches the stored cache at all: it either reuses a stored
// hash, refreshes one with a computed hash, or both. It is what decides whether a run needs a
// signature cache, and it is true for HashRead, which hashes content and writes nothing:
// reusesCache reports the lookup, refreshesCache reports the write.
func (p HashPolicy) usesCache() bool {
return p != HashOff
}