]> Cypherpunks repositories - gostls13.git/commitdiff
os: rewrite TestChtimesWithZeroTimes
authorKir Kolyshkin <kolyshkin@gmail.com>
Sun, 9 Jun 2024 19:47:23 +0000 (12:47 -0700)
committerGopher Robot <gobot@golang.org>
Wed, 26 Jun 2024 16:10:48 +0000 (16:10 +0000)
First, this enables checks on DragonFly BSD, which partially works since
CL 589496 (except two things: atime is not supported on hammer2 fs, and
when both times are omitted, it doesn't work due to a kernel bug).

Second, there are a few problems with TestChtimesWithZeroTimes:

 - test cases are interdependent (former cases influence the latter ones),
   making the test using too many different times and also hard to read;

 - time is changed forward not backward which could be racy;

 - if the test has failed, it hard to see which exact case is failing.

Plus, there are issues with the error exclusion code in
TestChtimesWithZeroTimes:

 - the atime comparison is done twice for the default ("unix") case;

 - the atime exclusion caused by noatime mount flag applies to all
   unixes rather than netbsd only as it should;

 - the atime exclusion tries to read wrong files (/bin/mounts and
   /etc/mtab instead of /proc/mounts);

 - the exclusion for netbsd is only applied for 64-bit arches, which
   seems wrong (and I've reproduced noatime issue on NetBSD 9.4/i386).

Let's rewrite it, fixing all these issues, and rename to
TestChtimesOmit.

NB: TestChtimes can now be removed.

Change-Id: If9020256ca920b4db836a1f0b2e055b5fce4a552
Reviewed-on: https://go-review.googlesource.com/c/go/+/591535
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joedian Reid <joedian@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/os/os_test.go

index b3fbe42ba788b8e416a43fc6fb6a687f0b0e9246..9519aa0fc697873ef35ddcb042fe2216f689f85e 100644 (file)
@@ -1383,123 +1383,108 @@ func TestChtimes(t *testing.T) {
        testChtimes(t, f.Name())
 }
 
-func TestChtimesWithZeroTimes(t *testing.T) {
+func TestChtimesOmit(t *testing.T) {
+       t.Parallel()
+
+       testChtimesOmit(t, true, false)
+       testChtimesOmit(t, false, true)
+       testChtimesOmit(t, true, true)
+       testChtimesOmit(t, false, false) // Same as TestChtimes.
+}
+
+func testChtimesOmit(t *testing.T, omitAt, omitMt bool) {
+       t.Logf("omit atime: %v, mtime: %v", omitAt, omitMt)
        file := newFile(t)
        _, err := file.Write([]byte("hello, world\n"))
        if err != nil {
-               t.Fatalf("Write: %s", err)
+               t.Fatal(err)
        }
-       fName := file.Name()
+       name := file.Name()
        err = file.Close()
        if err != nil {
-               t.Errorf("%v", err)
+               t.Error(err)
        }
-       fs, err := Stat(fName)
+       fs, err := Stat(name)
        if err != nil {
                t.Fatal(err)
        }
-       startAtime := Atime(fs)
-       startMtime := fs.ModTime()
+
+       wantAtime := Atime(fs)
+       wantMtime := fs.ModTime()
        switch runtime.GOOS {
        case "js":
-               startAtime = startAtime.Truncate(time.Second)
-               startMtime = startMtime.Truncate(time.Second)
+               wantAtime = wantAtime.Truncate(time.Second)
+               wantMtime = wantMtime.Truncate(time.Second)
        }
-       at0 := startAtime
-       mt0 := startMtime
-       t0 := startMtime.Truncate(time.Second).Add(1 * time.Hour)
 
-       tests := []struct {
-               aTime     time.Time
-               mTime     time.Time
-               wantATime time.Time
-               wantMTime time.Time
-       }{
-               {
-                       aTime:     time.Time{},
-                       mTime:     time.Time{},
-                       wantATime: startAtime,
-                       wantMTime: startMtime,
-               },
-               {
-                       aTime:     t0.Add(200 * time.Second),
-                       mTime:     time.Time{},
-                       wantATime: t0.Add(200 * time.Second),
-                       wantMTime: startMtime,
-               },
-               {
-                       aTime:     time.Time{},
-                       mTime:     t0.Add(100 * time.Second),
-                       wantATime: t0.Add(200 * time.Second),
-                       wantMTime: t0.Add(100 * time.Second),
-               },
-               {
-                       aTime:     t0.Add(300 * time.Second),
-                       mTime:     t0.Add(100 * time.Second),
-                       wantATime: t0.Add(300 * time.Second),
-                       wantMTime: t0.Add(100 * time.Second),
-               },
+       var setAtime, setMtime time.Time // Zero value means omit.
+       if !omitAt {
+               wantAtime = wantAtime.Add(-1 * time.Second)
+               setAtime = wantAtime
+       }
+       if !omitMt {
+               wantMtime = wantMtime.Add(-1 * time.Second)
+               setMtime = wantMtime
        }
 
-       for _, tt := range tests {
-               // Now change the times accordingly.
-               if err := Chtimes(fName, tt.aTime, tt.mTime); err != nil {
-                       t.Error(err)
-               }
+       // Change the times accordingly.
+       if err := Chtimes(name, setAtime, setMtime); err != nil {
+               t.Error(err)
+       }
 
-               // Finally verify the expectations.
-               fs, err = Stat(fName)
-               if err != nil {
-                       t.Error(err)
-               }
-               at0 = Atime(fs)
-               mt0 = fs.ModTime()
-
-               if got, want := at0, tt.wantATime; !got.Equal(want) {
-                       errormsg := fmt.Sprintf("AccessTime mismatch with values ATime:%q-MTime:%q\ngot:  %q\nwant: %q", tt.aTime, tt.mTime, got, want)
-                       switch runtime.GOOS {
-                       case "plan9":
-                               // Mtime is the time of the last change of
-                               // content.  Similarly, atime is set whenever
-                               // the contents are accessed; also, it is set
-                               // whenever mtime is set.
-                       case "windows":
+       // Verify the expectations.
+       fs, err = Stat(name)
+       if err != nil {
+               t.Error(err)
+       }
+       gotAtime := Atime(fs)
+       gotMtime := fs.ModTime()
+
+       if !gotAtime.Equal(wantAtime) {
+               errormsg := fmt.Sprintf("atime mismatch, got: %q, want: %q", gotAtime, wantAtime)
+               switch runtime.GOOS {
+               case "plan9":
+                       // Mtime is the time of the last change of content.
+                       // Similarly, atime is set whenever the contents are
+                       // accessed; also, it is set whenever mtime is set.
+               case "dragonfly":
+                       if omitAt && omitMt {
+                               t.Log(errormsg)
+                               t.Log("Known DragonFly BSD issue (won't work when both times are omitted); ignoring.")
+                       } else {
+                               // Assume hammer2 fs; https://www.dragonflybsd.org/hammer/ says:
+                               // > Because HAMMER2 is a block copy-on-write filesystem,
+                               // > the "atime" field is not supported and will typically
+                               // > just reflect local system in-memory caches or mtime.
+                               //
+                               // TODO: if only can CI define TMPDIR to point to a tmpfs
+                               // (e.g. /var/run/shm), this exception can be removed.
+                               t.Log(errormsg)
+                               t.Log("Known DragonFly BSD issue (atime not supported on hammer2); ignoring.")
+                       }
+               case "netbsd":
+                       if !omitAt && hasNoatime() {
+                               t.Log(errormsg)
+                               t.Log("Known NetBSD issue (atime not changed on fs mounted with noatime); ignoring.")
+                       } else {
                                t.Error(errormsg)
-                       default: // unix's
-                               if got, want := at0, tt.wantATime; !got.Equal(want) {
-                                       mounts, err := ReadFile("/bin/mounts")
-                                       if err != nil {
-                                               mounts, err = ReadFile("/etc/mtab")
-                                       }
-                                       if strings.Contains(string(mounts), "noatime") {
-                                               t.Log(errormsg)
-                                               t.Log("A filesystem is mounted with noatime; ignoring.")
-                                       } else {
-                                               switch runtime.GOOS {
-                                               case "netbsd", "dragonfly":
-                                                       // On a 64-bit implementation, birth time is generally supported and cannot be changed.
-                                                       // When supported, atime update is restricted and depends on the file system and on the
-                                                       // OS configuration.
-                                                       if strings.Contains(runtime.GOARCH, "64") {
-                                                               t.Log(errormsg)
-                                                               t.Log("Filesystem might not support atime changes; ignoring.")
-                                                       }
-                                               default:
-                                                       t.Error(errormsg)
-                                               }
-                                       }
-                               }
                        }
+               default:
+                       t.Error(errormsg)
                }
-               if got, want := mt0, tt.wantMTime; !got.Equal(want) {
-                       errormsg := fmt.Sprintf("ModTime mismatch with values ATime:%q-MTime:%q\ngot:  %q\nwant: %q", tt.aTime, tt.mTime, got, want)
-                       switch runtime.GOOS {
-                       case "dragonfly":
+       }
+       if !gotMtime.Equal(wantMtime) {
+               errormsg := fmt.Sprintf("mtime mismatch, got: %q, want: %q", gotMtime, wantMtime)
+               switch runtime.GOOS {
+               case "dragonfly":
+                       if omitAt && omitMt {
                                t.Log(errormsg)
-                               t.Log("Mtime is always updated; ignoring.")
-                       default:
+                               t.Log("Known DragonFly BSD issue (won't work when both times are omitted); ignoring.")
+                       } else {
                                t.Error(errormsg)
                        }
+               default:
+                       t.Error(errormsg)
                }
        }
 }