bisync: fix time.Local data race on tests - fixes #8272

Before this change, the bisync tests were directly setting the time.Local
variable to UTC.

The reason for overriding the time zone on the tests is to make them
deterministic regardless of where in the world the user happens to be. There are
some goldenized strings which have the time zone hard-coded and would result in a
miscompare failure outside of that time zone.

However, mutating the time.Local variable is not the right way to do this, as OP
correctly pointed out on #8272.

Setting the TZ environment variable from within the code was also not an ideal
solution because, while it worked on unix, it did not work on Windows. See
fbac94a799/src/time/zoneinfo.go (L79-L80)

This change fixes the issue by defining a new bisync.LogTZ setting for use when
printing timestamps in /cmd/bisync/resolve.go. We override this on the tests
instead of time.Local.

cmd/bisync/bisync_test.go

@@ -226,6 +226,7 @@ var color = bisync.Color
 
 // TestMain drives the tests
 func TestMain(m *testing.M) {
+	bisync.LogTZ = time.UTC
 	fstest.TestMain(m)
 }
 
@@ -277,7 +278,6 @@ func testBisync(t *testing.T, path1, path2 string) {
 		ci.RefreshTimes = true
 	}
 	bisync.Colors = true
-	time.Local = bisync.TZ
 	ci.FsCacheExpireDuration = fs.Duration(5 * time.Hour)
 
 	baseDir, err := os.Getwd()

cmd/bisync/listing.go

@@ -42,10 +42,14 @@ var lineRegex = regexp.MustCompile(`^(\S) +(-?\d+) (\S+) (\S+) (\d{4}-\d\d-\d\dT
 // timeFormat defines time format used in listings
 const timeFormat = "2006-01-02T15:04:05.000000000-0700"
 
-// TZ defines time zone used in listings
 var (
+	// TZ defines time zone used in listings
 	TZ      = time.UTC
 	tzLocal = false
+
+	// LogTZ defines time zone used in logs (which may be different than that used in listings).
+	// time.Local by default, but we force UTC on tests to make them deterministic regardless of tester's location.
+	LogTZ = time.Local
 )
 
 // fileInfo describes a file

cmd/bisync/resolve.go

@@ -380,26 +380,26 @@ func (b *bisyncRun) resolveNewerOlder(t1, t2 time.Time, remote1, remote2 string,
 	}
 	if t1.After(t2) {
 		if prefer == PreferNewer {
-			fs.Infof(remote1, "Path1 is newer. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t1.Sub(t2))
+			fs.Infof(remote1, "Path1 is newer. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t1.Sub(t2))
 			return 1
 		} else if prefer == PreferOlder {
-			fs.Infof(remote1, "Path2 is older. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t1.Sub(t2))
+			fs.Infof(remote1, "Path2 is older. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t1.Sub(t2))
 			return 2
 		}
 	} else if t1.Before(t2) {
 		if prefer == PreferNewer {
-			fs.Infof(remote1, "Path2 is newer. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t2.Sub(t1))
+			fs.Infof(remote1, "Path2 is newer. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t2.Sub(t1))
 			return 2
 		} else if prefer == PreferOlder {
-			fs.Infof(remote1, "Path1 is older. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t2.Sub(t1))
+			fs.Infof(remote1, "Path1 is older. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t2.Sub(t1))
 			return 1
 		}
 	}
 	if t1.Equal(t2) {
-		fs.Infof(remote1, "Winner cannot be determined as times are equal. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t2.Sub(t1))
+		fs.Infof(remote1, "Winner cannot be determined as times are equal. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t2.Sub(t1))
 		return 0
 	}
-	fs.Errorf(remote1, "Winner cannot be determined. Path1: %v, Path2: %v", t1.Local(), t2.Local()) // shouldn't happen unless prefer is of wrong type
+	fs.Errorf(remote1, "Winner cannot be determined. Path1: %v, Path2: %v", t1.In(LogTZ), t2.In(LogTZ)) // shouldn't happen unless prefer is of wrong type
 	return 0
 }