From a280ac6205edd832333b4e05fb6bda301d8f80e6 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 7 Sep 2026 12:56:55 +0200 Subject: [PATCH] fix: one directory holds one run at a time Two runs writing into one directory used to write over each other's files and both report that they had succeeded. Measured with two processes started on the same wall clock instant, sixty files of 200 kB each, eight times: both ended 0 2 of 8 120 files reported, 60 on the disk both ended 8 5 of 8 partial, and only because Windows refuses to rename onto a file another process holds open one ended 5 1 of 8 the preflight happened to see the other run In the first case verify against the first run's manifest reported sixty wrong hashes about a run that had been told it succeeded. The middle case is not a defence: it is a property of one system, and Linux renames onto a file another process has open without complaint. The protection already existed and was keyed to the wrong thing. A run claims its manifest name before the first file and keeps it until it ends, so two runs both writing manifest.json into one directory have always been refused - measured the same day, four times out of four. output.manifest is the one way out of that claim, and it was never meant to be a way out of this. So this widens the key of the claim that is already there rather than adding a second mechanism. A run takes core.RunLockName in the output directory, holds it for its whole length, and gives it back however it ends, interruption included. The preflight asks about it before it asks about the manifest name, which is what a dry run needs, and is also the better sentence: a second run used to be told that manifest.json already exists and is the only record of what an earlier run wrote, about a run that was still going. Eight of eight refused after the change. verify names the lock as ours rather than as a file somebody else left, which makes it the third marker to need that repair after the partial and the writing ones. Its sentence holds both endings open, because it is the only one of the three that may belong to a run still in flight. The cost is stated in the refusal, in the changelog and in the document. A run killed outright cannot give the name back, so the next run into that directory is refused until a person deletes the file. And two runs can no longer fill one directory at the same time, even when the names they write do not collide - for every run that does not set output.manifest, that was already true. Six guards, six mutations. Two of the mutations found faults in the guards rather than in the code: the stopped-run guard cancelled BEFORE the run, so the lock was never taken and "it is gone afterwards" was true of nothing, and a seventh guard proved exactly what the first one proves, because the lock never sees the manifest name at all. Co-authored-by: Claude Opus 5 --- CHANGELOG.md | 27 +++ internal/audit/audit.go | 29 +++- internal/cli/errors.go | 8 + internal/core/limits.go | 38 +++++ internal/engine/engine.go | 50 ++++++ internal/engine/errors.go | 23 +++ internal/engine/preflight.go | 30 ++++ internal/guard/runlock_test.go | 290 +++++++++++++++++++++++++++++++++ 8 files changed, 489 insertions(+), 6 deletions(-) create mode 100644 internal/guard/runlock_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bf34e6..c98fe13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,33 @@ because it turns other people's test suites red. ### Fixed +- **Two runs writing into one directory can no longer write over each other's + files.** A run holds the directory it is writing into for as long as it is + writing. A second run that starts meanwhile is refused before it writes a + byte, with exit code `5` and a message saying so. + + That protection was already there and was attached to the wrong thing. A run + takes its manifest name at the start and keeps it until it ends, so two runs + both writing `manifest.json` into one directory have always been refused. A + recipe that points `output.manifest` at a name of its own had nothing holding + it. Measured with two runs started on the same instant, eight times: twice + both ended `0`, both reported sixty files produced, and sixty files existed - + every one of the first run's replaced by the second run's, without either run + saying anything. `tfg verify` against the first manifest then reported sixty + changed files for a run that had been told it succeeded. + + What holds the directory is a file called `.tfg-run-lock`. The run removes it + when it ends, including when you stop it with Ctrl+C. A run killed outright + cannot remove it, so it stays behind and the next run into that directory is + refused until you delete it - the refusal names the file and says exactly + that. `tfg verify` also names it, as a leftover of ours rather than as a file + somebody else put there, and `tfg cleanup` will not remove it, because it + removes only what a manifest lists. + + The cost is worth stating plainly: two runs can no longer fill one directory + at the same time, even when the files they write have different names. For + every run that does not set `output.manifest` that was already true. + - **The About screen now shows the support address, so the Donate button is no longer the only way to reach it.** Pressing Donate asks your desktop to open the page. On a machine with no browser registered that quietly does nothing, diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 0930872..831bc53 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -95,6 +95,18 @@ func (d Difference) String() string { case Unreadable: return fmt.Sprintf("unreadable %s - %s", d.Path, d.Got) case Leftover: + // The lock a run holds while it writes into a directory, and it is the + // one of these that may belong to something still going on. So the + // sentence says both endings rather than asserting the run is dead: + // verify cannot tell, and telling somebody to delete the mark a live + // run is holding would let a second run in behind it. + if core.IsRunLockName(filepath.Base(d.Path)) { + return fmt.Sprintf( + "leftover %s\n the mark a run holds while it writes into this directory. If a run is going "+ + "on it will remove this itself when it ends. If none is, it was killed before it could tidy up - "+ + "no files were lost, but no new run will start here until this is deleted by hand", + d.Path) + } // Two markers reach this, and one sentence cannot serve both. The // first is a file that was being produced, so nothing is lost. The // second is a RECORD that was being saved, so the useful thing to say @@ -262,12 +274,17 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) kind := Extra want := "" switch { - case core.IsPartialName(filepath.Base(p)), core.IsWritingName(filepath.Base(p)): - // Both markers, because both name a file this tool started and did - // not finish. Only the first was recognised until 2026-09-06, so a - // half written manifest was reported as "extra" - the word that - // means somebody else put it here. They get different sentences in - // String, because what a reader should do about them differs. + case core.IsPartialName(filepath.Base(p)), + core.IsWritingName(filepath.Base(p)), + core.IsRunLockName(filepath.Base(p)): + // All three markers, because each names something this tool put + // here and did not take away. Only the first was recognised until + // 2026-09-06, so a half written manifest was reported as "extra" - + // the word that means somebody else put it here - and the third + // arrived with the run lock on 2026-09-07. They get different + // sentences in String, because what a reader should do about them + // differs, and about the lock it differs most: it is the only one + // that may belong to a run that is still going. kind = Leftover default: // One file under two spellings reads as a polluted directory diff --git a/internal/cli/errors.go b/internal/cli/errors.go index 6882c80..7c84f06 100644 --- a/internal/cli/errors.go +++ b/internal/cli/errors.go @@ -274,6 +274,14 @@ func classifyReading(err error) (int, bool) { if errors.As(err, &collision) { return ExitIO, true } + // A directory somebody else's run is holding. The same code as a name that + // is taken, because it is the same kind of answer - the disk would not have + // this run - and the frozen table has one row for that. What differs is the + // sentence, and that lives on the error. + var inProgress *engine.RunInProgressError + if errors.As(err, &inProgress) { + return ExitIO, true + } // A manifest we cannot read is a reading failure, not a bug in the tool. // Falling through to RUNTIME would tell CI to file a report against us for // a file somebody handed in. diff --git a/internal/core/limits.go b/internal/core/limits.go index 5ac3e88..dca2e64 100644 --- a/internal/core/limits.go +++ b/internal/core/limits.go @@ -150,6 +150,44 @@ func IsWritingName(name string) bool { return strings.HasSuffix(name, WritingMarker) } +// RunLockName is the name a run holds for as long as it is writing into a +// directory. +// +// The third of these, and the only one that is a whole name rather than a +// suffix on somebody else's: it belongs to the run, not to a file. A run takes +// it exclusively before the first byte and gives it back when it ends, so a +// second run starting into the same directory is refused rather than allowed +// to write over what the first one is producing. +// +// Why it had to exist, measured on 2026-09-07 with two runs started on the +// same wall clock instant, eight times: twice both runs ended 0, each said it +// had produced sixty files, and sixty files were on the disk - every one of +// the first run's belonging to the second. Five times the runs ended 8, and +// that was luck rather than a defence: Windows refuses to rename onto a file +// another process holds open, which is not something Linux does. +// +// The protection this restores already existed and was keyed to the wrong +// thing. A run claims its manifest name for its whole length, so two runs +// writing manifest.json into one directory have always been refused - measured +// the same day, four times out of four. output.manifest was the one way out of +// that, and it was never meant to be a way out of this. +// +// Declared here beside the other two for the reason written above them: two +// parts of the tool have to agree on the spelling. The engine writes it, and +// verify has to recognise one that outlived its run rather than call it a file +// somebody else put there. +const RunLockName = ".tfg-run-lock" + +// IsRunLockName says whether a name is that lock. +// +// A whole name rather than a suffix, so this is equality rather than a search. +// Written as a function anyway, because every reader of the other two markers +// asks through one and a reader that compares the constant itself is a reader +// that will not be found when the spelling changes. +func IsRunLockName(name string) bool { + return name == RunLockName +} + // AddSizes adds one file size to a running total and says when the total has // left the range it is measured in. // diff --git a/internal/engine/engine.go b/internal/engine/engine.go index c0cf03c..73bb581 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -519,6 +519,31 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) return res, fmt.Errorf("cannot create the output directory %s: %w", opt.OutDir, err) } + // The directory is taken before the manifest name is, and the two are not + // the same claim. This one does not depend on what the manifest is called, + // so a recipe that points output.manifest somewhere of its own is held by + // it exactly as a default run is. + // + // That difference is the whole of it. Measured on 2026-09-07 with two runs + // started on the same instant, eight times: twice both ended 0, both said + // sixty files, and sixty files existed - the second run's, over the first + // run's names. verify against the first manifest then reported sixty wrong + // hashes for a run that had been told it succeeded. With the same manifest + // name the claim below already refused that, four times out of four, which + // is why this is the same mechanism rather than a new one. + lockPath := RunLockPath(opt.OutDir) + if err := claimRunLock(lockPath); err != nil { + if errors.Is(err, fs.ErrExist) { + return res, &RunInProgressError{Path: lockPath, Dir: opt.OutDir} + } + return res, fmt.Errorf("cannot start a run in %s: %w", opt.OutDir, err) + } + // Given back however this run ends, including one stopped part way: the + // signal cancels the context, Run returns, and this runs. What it cannot + // cover is the process being killed outright, and that is why the refusal + // above names the file to remove. + defer releaseRunLock(lockPath) + // The manifest name is taken before the first file, not after the last one. // // Claiming it at save time already stopped two runs from both writing a @@ -611,6 +636,31 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) return res, nil } +// claimRunLock takes the name that says this directory has a run in it. +// +// core.CreateNew rather than os.Create, and that is the whole claim: it refuses +// a name something already holds and it believes the refusal only when Lstat +// finds something there, so a directory reached through a link still works. +// The file stays empty - the same shape as the manifest claim, and for the same +// reason. Nothing reads it, so there is nothing in it to be read half written. +func claimRunLock(path string) error { + fh, err := core.CreateNew(path, 0o666) + if err != nil { + return err + } + return fh.Close() +} + +// releaseRunLock gives the name back. +// +// The failure is dropped on purpose. A run that finished and could not remove +// its own lock has nothing useful to say to the person - the files are written +// and the manifest is saved - and the next run into that directory will name +// the file and say what to do about it. +func releaseRunLock(path string) { + _ = os.Remove(path) +} + func entryFor(f PlannedFile, sha string, materialized bool, failure error) manifest.File { var notes []manifest.Note for _, n := range f.Plan.Notes { diff --git a/internal/engine/errors.go b/internal/engine/errors.go index 08917b4..7c57231 100644 --- a/internal/engine/errors.go +++ b/internal/engine/errors.go @@ -208,6 +208,29 @@ func (e *SpaceError) Error() string { e.Needed, e.Path, e.Available) } +// RunInProgressError is refusing to start because another run holds this +// directory. +// +// Its own type rather than a third state on CollisionError, because it is a +// different fault with a different remedy. A collision is a name that is taken +// and the answer is to move that file or write somewhere else. This is a run +// that has not finished, and the answer is usually to wait. +// +// The last sentence is there because a run killed outright cannot give the +// name back, and a person who is told only "another run is writing here" about +// a machine where nothing is running has been given a dead end. It names the +// file so that clearing it is one command rather than a hunt. +type RunInProgressError struct { + Path string + Dir string +} + +func (e *RunInProgressError) Error() string { + return fmt.Sprintf( + "another run is already writing into %s, so this one will not start. Two runs writing into one directory can write over each other's files without either of them saying so. Wait for it to finish, or generate into a different directory. If nothing is running, that run was killed before it could tidy up - remove %s and try again", + e.Dir, e.Path) +} + // CollisionError is refusing to write over something that is already there. // // This tool runs in directories that belong to the user. Overwriting without diff --git a/internal/engine/preflight.go b/internal/engine/preflight.go index ca85e4a..c8ad74b 100644 --- a/internal/engine/preflight.go +++ b/internal/engine/preflight.go @@ -54,6 +54,22 @@ func preflight(ctx context.Context, files []PlannedFile, opt Options) error { Remedy: "Point the output directory at a directory, or at one that does not exist yet and it will be created"} } + // Whether anybody else is writing here comes before every other question + // about a name, and the order is the message rather than tidiness. A run in + // flight has already claimed its manifest name, so without this check a + // second run into the same directory was told "manifest.json already exists + // ... it is the only record of what an earlier run wrote" - a sentence + // about a run that finished, said about one that is still going, which + // sends somebody looking through a directory rather than waiting a minute. + // + // Asked here rather than only at the claim below because a dry run stops + // before the claim. A preview that says a run would succeed, while another + // run is filling the directory it would write into, is answering a question + // nobody asked. + if path := RunLockPath(opt.OutDir); exists(path) { + return &RunInProgressError{Path: path, Dir: opt.OutDir} + } + // The manifest is checked with the files it would describe, and leaving it // out cost exactly what it protects. A second run into the same directory // wrote a fresh manifest over the old one, so every file the old one listed @@ -205,3 +221,17 @@ func manifestNameOf(opt Options) string { func ManifestPath(opt Options) string { return filepath.Join(opt.OutDir, manifestNameOf(opt)) } + +// RunLockPath is the name a run holds while it writes into a directory. +// +// Exported for the same reason ManifestPath is: more than one part of the tool +// has to mean the same file. The check above asks whether it is taken, the run +// takes it, and a guard has to be able to put one there and watch a second run +// refuse. +// +// It does not depend on the manifest name, and that independence is the whole +// point. Two runs pointing output.manifest at different files are two runs +// writing into one directory, which is the case the manifest claim cannot see. +func RunLockPath(outDir string) string { + return filepath.Join(outDir, core.RunLockName) +} diff --git a/internal/guard/runlock_test.go b/internal/guard/runlock_test.go new file mode 100644 index 0000000..5f643e5 --- /dev/null +++ b/internal/guard/runlock_test.go @@ -0,0 +1,290 @@ +package guard + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/audit" + "github.com/donislawdev/TestingFilesGenerator/internal/core" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" +) + +// Two runs writing into one directory used to write over each other's files +// and both say they had succeeded. +// +// Measured on 2026-09-07 with two processes started on the same wall clock +// instant, eight times, sixty files of 200 kB each: +// +// both ended 0 2 of 8 120 files reported, 60 on the disk +// both ended 8 5 of 8 partial, and only because Windows refuses to +// rename onto a file another process holds open +// one ended 5 1 of 8 the preflight happened to see the other run +// +// In the first case verify against the first run's manifest reported sixty +// wrong hashes about a run that had been told it succeeded. The second case is +// not a defence: it is a property of one filesystem on one system, and Linux +// renames onto an open file without complaint. +// +// The protection existed and was keyed to the wrong thing. A run claims its +// manifest name for its whole length, so two runs both writing manifest.json +// into one directory have always been refused - measured the same day, four +// times out of four. output.manifest is the one way out of that claim, and it +// was never meant to be a way out of this. +// +// None of the guards below start two processes. They reproduce what two +// processes meet, which is a directory that is already held, and assert on it +// directly - a race somebody has to lose to see fail is not a guard. + +// planIn is one run's worth of files, planned into dir. +// +// The count is a parameter because one guard here needs a run long enough to +// be interrupted in the middle of, and four small files is not that. +func planIn(t *testing.T, dir, manifestName string, count int) ([]engine.PlannedFile, engine.Options) { + t.Helper() + opt := engine.Options{OutDir: dir, ManifestName: manifestName, Seed: 4242, Command: "test"} + planned, err := engine.Plan([]engine.Target{txtTarget("files", count, 2048)}, opt) + if err != nil { + t.Fatalf("planning: %v", err) + } + return planned, opt +} + +// The run this refuses points output.manifest at a name of its own, and that +// is the case rather than an incidental detail. It is the one output.manifest +// was letting through, and naming a free manifest here means the older claim +// cannot be what refuses - which is asserted below rather than assumed. +// +// It was two guards until the mutation coverage said otherwise. The second +// one used a non-default manifest name and the first the default, and every +// mutation that reddens one reddens the other, because the lock never sees the +// manifest name at all. Two guards proving one thing is a coverage number that +// is larger than the coverage. +func TestASecondRunIntoADirectoryARunIsHoldingIsRefusedBeforeItWritesAnything(t *testing.T) { + dir := t.TempDir() + lock := engine.RunLockPath(dir) + if err := os.WriteFile(lock, nil, 0o644); err != nil { + t.Fatalf("standing in for the run that is already here: %v", err) + } + + const ownName = "manifest-beta.json" + if _, err := os.Stat(filepath.Join(dir, ownName)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("%s is not free, so this guard could pass on the manifest claim alone and say nothing about the directory", ownName) + } + + planned, opt := planIn(t, dir, ownName, 4) + res, err := engine.Run(context.Background(), planned, opt) + + var inProgress *engine.RunInProgressError + if !errors.As(err, &inProgress) { + t.Fatalf("a run started into a directory another run is holding, and got %v.\n"+ + "Two runs writing into one directory write over each other's files, and both of them say they succeeded", err) + } + if res.Started { + t.Error("the refused run says it started, so its caller will go on to write a manifest for files that do not exist") + } + + // Refused BEFORE anything was written, which is the half that makes the + // refusal worth having. A run that stops after eight files has already + // taken eight names off somebody. + if got := namesIn(t, dir); len(got) != 1 || got[0] != core.RunLockName { + t.Errorf("the refused run left %v in the directory - it has to refuse before it writes anything", got) + } + + // The remedy is in the sentence, because a run killed outright cannot give + // the name back and "another run is writing here" on a machine where + // nothing runs is a dead end. + if !strings.Contains(inProgress.Error(), core.RunLockName) { + t.Errorf("the refusal does not name the file to remove:\n %s", inProgress.Error()) + } +} + +// Held while the run writes, and given back when it ends. +// +// The first half is asked from inside the run rather than around it, because +// "the lock is gone afterwards" is also true of a lock that was never taken. +func TestTheDirectoryIsHeldWhileTheRunWritesAndGivenBackWhenItEnds(t *testing.T) { + dir := t.TempDir() + planned, opt := planIn(t, dir, "manifest.json", 4) + + held := false + asked := false + opt.OnProgress = func(engine.Progress) { + asked = true + if _, err := os.Stat(engine.RunLockPath(dir)); err == nil { + held = true + } + } + + if _, err := engine.Run(context.Background(), planned, opt); err != nil { + t.Fatalf("running: %v", err) + } + if !asked { + t.Fatal("the run never reported progress, so nothing looked while it was writing and this guard checked nothing") + } + if !held { + t.Error("the directory was not held while the run was writing, so a second run starting in the middle of this one would be let in") + } + if _, err := os.Stat(engine.RunLockPath(dir)); !errors.Is(err, os.ErrNotExist) { + t.Error("the finished run kept the directory, so nothing can ever generate into it again without a person deleting a file") + } +} + +// A run stopped part way gives the directory back too. +// +// It is the ending this cannot afford to get wrong: Ctrl+C is ordinary, and a +// tool that locked a directory every time somebody changed their mind would be +// worse than the fault it fixes. +// +// Stopped from INSIDE the run, and the first version of this was stopped +// before it. Measured 2026-09-07: with the context already cancelled the +// preflight refuses, the run never reaches the claim, and "the lock is gone" +// is true because it was never taken - so removing the release left this guard +// green. The mutation is what said so. It now cancels on the first report of +// progress, and asserts the lock was HELD at that moment rather than assuming +// it. +func TestAStoppedRunGivesTheDirectoryBack(t *testing.T) { + dir := t.TempDir() + planned, opt := planIn(t, dir, "manifest.json", 400) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + held := false + opt.OnProgress = func(engine.Progress) { + if _, err := os.Stat(engine.RunLockPath(dir)); err == nil { + held = true + } + cancel() + } + + _, err := engine.Run(ctx, planned, opt) + if !held { + t.Fatal("the run never held the directory while it was writing, so this guard is not looking at a stopped run that had it") + } + if !errors.Is(err, context.Canceled) { + t.Fatalf("the run was cancelled from inside itself and ended with %v - this guard needs a run long enough to be stopped in the middle of", err) + } + if _, err := os.Stat(engine.RunLockPath(dir)); !errors.Is(err, os.ErrNotExist) { + t.Error("a stopped run kept the directory - every interrupted run would leave one that a person has to delete by hand") + } +} + +// verify names it as ours. +// +// Extra means somebody else put it here, and about our own mark that sends a +// person looking for whoever polluted their directory. It is the same repair +// the two older markers already had, arriving a third time. +func TestVerifyNamesTheRunLockAsOursRatherThanAsSomebodyElses(t *testing.T) { + dir := t.TempDir() + planned, opt := planIn(t, dir, "manifest.json", 4) + res, err := engine.Run(context.Background(), planned, opt) + if err != nil { + t.Fatalf("running: %v", err) + } + if err := os.WriteFile(engine.RunLockPath(dir), nil, 0o644); err != nil { + t.Fatalf("standing in for a run that was killed: %v", err) + } + + diffs, err := audit.Verify(context.Background(), dir, res.Manifest, "manifest.json") + if err != nil { + t.Fatalf("verifying: %v", err) + } + if len(diffs) != 1 { + t.Fatalf("expected the lock and nothing else, got %v", diffs) + } + if diffs[0].Kind != audit.Leftover { + t.Errorf("verify called our own run lock %q - that word means somebody else put it here", diffs[0].Kind) + } + // The sentence has to hold both endings open. A live run is holding one of + // these too, and telling somebody to delete it would let a second run in + // behind the first. + said := diffs[0].String() + for _, phrase := range []string{"If a run is going on", "If none is"} { + if !strings.Contains(said, phrase) { + t.Errorf("the sentence about the lock does not say what to do when a run IS going on:\n %s", said) + } + } +} + +// The claim is empty, and stays empty. +// +// The manifest claim carries the same rule for a reason written where it is +// made: a claim with something in it is a claim somebody's reader will take +// for a document. Nothing reads the lock, so there is nothing in it that could +// be read half written - and that is a decision worth pinning rather than +// rediscovering when somebody wants to put a process id in it. +func TestTheRunLockCarriesNothing(t *testing.T) { + dir := t.TempDir() + planned, opt := planIn(t, dir, "manifest.json", 4) + + var size int64 = -1 + opt.OnProgress = func(engine.Progress) { + if info, err := os.Stat(engine.RunLockPath(dir)); err == nil { + size = info.Size() + } + } + if _, err := engine.Run(context.Background(), planned, opt); err != nil { + t.Fatalf("running: %v", err) + } + if size < 0 { + t.Fatal("the lock was never looked at while the run was going, so this guard checked nothing") + } + if size != 0 { + t.Errorf("the run lock carries %d B - it is a claim on a name, and anything in it is something a reader could find half written", size) + } +} + +// A dry run does not take the directory, and it does not lie about one that is +// taken. +// +// Both halves are the same decision seen from two sides. A preview writes +// nothing, so it has no business holding a name - and a preview that says a +// run would succeed, while another run is filling the directory it would write +// into, is answering a question nobody asked. +func TestADryRunNeitherTakesTheDirectoryNorIgnoresIt(t *testing.T) { + dir := t.TempDir() + planned, opt := planIn(t, dir, "manifest.json", 4) + opt.DryRun = true + + if _, err := engine.Run(context.Background(), planned, opt); err != nil { + t.Fatalf("a dry run into an empty directory: %v", err) + } + if got := namesIn(t, dir); len(got) != 0 { + t.Errorf("a dry run left %v behind - it is supposed to write nothing at all", got) + } + + if err := os.WriteFile(engine.RunLockPath(dir), nil, 0o644); err != nil { + t.Fatalf("standing in for the run that is already here: %v", err) + } + var inProgress *engine.RunInProgressError + if _, err := engine.Run(context.Background(), planned, opt); !errors.As(err, &inProgress) { + t.Errorf("a dry run into a directory another run is holding reported %v.\n"+ + "The preview is the step this project tells people to take before anything large, so it has to give the answer the run would give", err) + } +} + +// The lock is spelled in one place. +// +// The same rule the other two markers carry, and for the same measured reason: +// a second spelling means the writing side and the reading side stop agreeing, +// and verify starts calling our own file somebody else's. +func TestTheRunLockIsSpelledInOnePlace(t *testing.T) { + dir := t.TempDir() + if engine.RunLockPath(dir) != filepath.Join(dir, core.RunLockName) { + t.Error("the engine builds the lock path from something other than core.RunLockName, so the two can drift apart") + } + if !core.IsRunLockName(core.RunLockName) { + t.Error("the reader does not recognise the name the writer uses") + } + if core.IsRunLockName(core.RunLockName + ".txt") { + t.Error("the reader recognises a name that only starts with the lock, so a file somebody else left would be called ours") + } + // Not a manifest name, or a run would refuse itself. + if core.RunLockName == engine.DefaultManifestName { + t.Error("the lock and the default manifest are the same name, so no run could ever start") + } +}