Changes to better support random seeds for flaky tests.

s2/cell_test.go

@@ -16,7 +16,6 @@ package s2
 
 import (
 	"math"
-	"math/rand"
 	"testing"
 	"unsafe"
 
@@ -559,9 +558,8 @@ func TestCellContainsPoint(t *testing.T) {
 }
 
 func TestCellContainsPointConsistentWithS2CellIDFromPoint(t *testing.T) {
-	// About 1% flaky with a random seed.
-	// TODO: https://github.com/golang/geo/issues/120
-	rand.Seed(1)
+	// TODO: Is it still about 1% flaky with a random seed.
+	// TODO(rsned): https://github.com/golang/geo/issues/120
 
 	// Construct many points that are nearly on a Cell edge, and verify that
 	// CellFromCellID(cellIDFromPoint(p)).Contains(p) is always true.
@@ -571,7 +569,7 @@ func TestCellContainsPointConsistentWithS2CellIDFromPoint(t *testing.T) {
 		i2 := (i1 + 1) & 3
 		v1 := cell.Vertex(i1)
 		v2 := samplePointFromCap(CapFromCenterAngle(cell.Vertex(i2), s1.Angle(epsilon)))
-		p := Interpolate(randomFloat64(), v1, v2)
+		p := Interpolate(randomUniformFloat64(0, 1.0), v1, v2)
 		if !CellFromCellID(cellIDFromPoint(p)).ContainsPoint(p) {
 			t.Errorf("For p=%v, CellFromCellID(cellIDFromPoint(p)).ContainsPoint(p) was false", p)
 		}
@@ -732,9 +730,8 @@ func maxDistanceToEdgeBruteForce(cell Cell, a, b Point) s1.ChordAngle {
 }
 
 func TestCellDistanceToEdge(t *testing.T) {
-	// About 0.1% flaky with a random seed.
-	// TODO: https://github.com/golang/geo/issues/120
-	rand.Seed(2)
+	// TODO: Is it still about 0.1% flaky with a random seed.
+	// TODO(rsned): https://github.com/golang/geo/issues/120
 
 	for iter := 0; iter < 1000; iter++ {
 		cell := CellFromCellID(randomCellID())
@@ -814,6 +811,6 @@ func TestCellMaxDistanceToCell(t *testing.T) {
 	}
 }
 
-// TODO(roberts): Differences from C++.
+// TODO(rsned): Differences from C++.
 // CellVsLoopRectBound
 // RectBoundIsLargeEnough

s2/cellunion_test.go

@@ -294,11 +294,11 @@ func TestCellUnion(t *testing.T) {
 }
 
 func addCells(id CellID, selected bool, input *[]CellID, expected *[]CellID, t *testing.T) {
-	// Decides whether to add "id" and/or some of its descendants to the test case.  If "selected"
-	// is true, then the region covered by "id" *must* be added to the test case (either by adding
-	// "id" itself, or some combination of its descendants, or both).  If cell ids are to the test
-	// case "input", then the corresponding expected result after simplification is added to
-	// "expected".
+	// Decides whether to add "id" and/or some of its descendants to the test case.
+	// If "selected" is true, then the region covered by "id" *must* be added to
+	// the test case (either by adding "id" itself, or some combination of its
+	// descendants, or both). If cell ids are to the test case "input", then the
+	// corresponding expected result after simplification is added to "expected".
 
 	if id == 0 {
 		// Initial call: decide whether to add cell(s) from each face.
@@ -309,8 +309,8 @@ func addCells(id CellID, selected bool, input *[]CellID, expected *[]CellID, t *
 	}
 
 	if id.IsLeaf() {
-		// The oneIn() call below ensures that the parent of a leaf cell will always be selected (if
-		// we make it that far down the hierarchy).
+		// The oneIn() call below ensures that the parent of a leaf cell will
+		// always be selected (if we make it that far down the hierarchy).
 		if selected != true {
 			t.Errorf("id IsLeaf() and not selected")
 		}
@@ -320,12 +320,14 @@ func addCells(id CellID, selected bool, input *[]CellID, expected *[]CellID, t *
 
 	// The following code ensures that the probability of selecting a cell at each level is
 	// approximately the same, i.e. we test normalization of cells at all levels.
+	// TODO(rsned): Change this probability to match
+	// absl::Bernoulli(bitgen, 1.0 / (MaxLevel - id.level()))
 	if !selected && oneIn(MaxLevel-id.Level()) {
-		//  Once a cell has been selected, the expected output is predetermined.  We then make sure
-		//  that cells are selected that will normalize to the desired output.
+		// Once a cell has been selected, the expected output is predetermined.
+		// We then make sure that cells are selected that will normalize to
+		// the desired output.
 		*expected = append(*expected, id)
 		selected = true
-
 	}
 
 	// With the rnd.OneIn() constants below, this function adds an average
@@ -340,6 +342,7 @@ func addCells(id CellID, selected bool, input *[]CellID, expected *[]CellID, t *
 		*input = append(*input, id)
 		added = true
 	}
+
 	numChildren := 0
 	for child := id.ChildBegin(); child != id.ChildEnd(); child = child.Next() {
 		// If the cell is selected, on average we recurse on 4/12 = 1/3 child.
@@ -360,20 +363,23 @@ func addCells(id CellID, selected bool, input *[]CellID, expected *[]CellID, t *
 			addCells(child, selected, input, expected, t)
 			numChildren++
 		}
+
 		// If this cell was selected but the cell itself was not added, we
 		// must ensure that all 4 children (or some combination of their
 		// descendants) are added.
-
 		if selected && !added {
 			addCells(child, selected, input, expected, t)
 		}
 	}
 }
 
+// TODO(rsned): This test has been split out into several smaller test cases in C++.
+// It might make sense to refactor this to stay more inline with what's current.
 func TestCellUnionNormalizePseudoRandom(t *testing.T) {
-	// About 2.4% flaky with a random seed.
-	// TODO: https://github.com/golang/geo/issues/120
-	rand.Seed(2)
+	// TODO(rsned): https://github.com/golang/geo/issues/120
+	// Test is still flaky without a fixed seed. Specify one for now until
+	// issue 120 is fully resolved.
+	random = rand.New(rand.NewSource(1))
 
 	// Try a bunch of random test cases, and keep track of average statistics
 	// for normalization (to see if they agree with the analysis above).

s2/convex_hull_query_test.go

@@ -16,7 +16,6 @@ package s2
 
 import (
 	"math"
-	"math/rand"
 	"testing"
 
 	"github.com/golang/geo/s1"
@@ -194,9 +193,8 @@ func TestConvexHullQueryLoopsAroundNorthPole(t *testing.T) {
 }
 
 func TestConvexHullQueryPointsInsideHull(t *testing.T) {
-	// About 0.3% flaky with a random seed.
-	// TODO: https://github.com/golang/geo/issues/120
-	rand.Seed(1)
+	// TODO: Verify if it's still about 0.3% flaky with a random seed.
+	// TODO(rsned): https://github.com/golang/geo/issues/120
 
 	// Repeatedly build the convex hull of a set of points, then add more points
 	// inside that loop and build the convex hull again. The result should
@@ -223,7 +221,7 @@ func TestConvexHullQueryPointsInsideHull(t *testing.T) {
 		// test pass reliably it means that we need to reject convex hulls whose
 		// bounding cap (when computed from a bounding rectangle) is not convex.
 		//
-		// TODO(roberts): This test can still fail (about 1 iteration in 500,000)
+		// TODO(rsned): This test can still fail (about 1 iteration in 500,000)
 		// because the Rect.CapBound implementation does not guarantee
 		// that A.Contains(B) implies A.CapBound().Contains(B.CapBound()).
 		if hull.CapBound().Height() >= 1 {

s2/edge_clipping_test.go

@@ -17,7 +17,6 @@ package s2
 import (
 	"fmt"
 	"math"
-	"math/rand"
 	"testing"
 
 	"github.com/golang/geo/r1"
@@ -248,9 +247,8 @@ func testClipToPaddedFace(t *testing.T, a, b Point) {
 }
 
 func TestEdgeClippingClipToPaddedFace(t *testing.T) {
-	// About 1.2% flaky with a random seed.
-	// TODO: https://github.com/golang/geo/issues/120
-	rand.Seed(1)
+	// TODO: Verify if it's still about 1.2% flaky with a random seed.
+	// TODO(rsned): https://github.com/golang/geo/issues/120
 
 	// Start with a few simple cases.
 	// An edge that is entirely contained within one cube face:

s2/edge_query_test.go

@@ -242,13 +242,17 @@ func testEdgeQueryWithGenerator(t *testing.T,
 	var indexCaps []Cap
 	var indexes []*ShapeIndex
 	for i := 0; i < numIndexes; i++ {
+		// TODO(rsned): Replace with:
+	        // r := rand.New(rand.NewSource(i))
 		rand.Seed(int64(i))
 		indexCaps = append(indexCaps, CapFromCenterAngle(randomPoint(), testCapRadius))
 		indexes = append(indexes, NewShapeIndex())
 		gen(indexCaps[i], numEdges, indexes[i])
 	}
 
 	for i := 0; i < numQueries; i++ {
+		// TODO(rsned): Replace with:
+	        // r := rand.New(rand.NewSource(i))
 		rand.Seed(int64(i))
 		iIndex := randomUniformInt(numIndexes)
 		indexCap := indexCaps[iIndex]
@@ -332,7 +336,7 @@ func testEdgeQueryWithGenerator(t *testing.T,
 //   - If maxErrorFraction > 0, then MaxError is set to the given
 //     fraction of the index radius.
 //
-// TODO(roberts): If there is a need to benchmark Furthest as well, this will need
+// TODO(rsned): If there is a need to benchmark Furthest as well, this will need
 // some changes to not use just the Closest variants of parts.
 // Furthest isn't doing anything different under the covers than Closest, so there
 // isn't really a huge need for benchmarking both.
@@ -360,12 +364,12 @@ func benchmarkEdgeQueryFindClosest(b *testing.B, bmOpts *edgeQueryBenchmarkOptio
 		bmOpts.numIndexEdges *= 4
 		b.Run(fmt.Sprintf("%d", bmOpts.numIndexEdges),
 			func(b *testing.B) {
-				// TODO(roberts): Return value 2 here is the slice of target
+				// TODO(rsned): Return value 2 here is the slice of target
 				// ShapeIndexes. Incorporate it once ShapeIndexTargets
 				// are able to be used in tests.
 				targets, _ = generateEdgeQueryWithTargets(bmOpts, query, index)
 				for i := 0; i < b.N; i++ {
-					// TODO(roberts): In the reference C++ benchmark
+					// TODO(rsned): In the reference C++ benchmark
 					// they use the tooling to split the benchmark
 					// run iterations up into kNumIndexSamples (8)
 					// times and pause to generate a new geometry
@@ -433,6 +437,7 @@ func generateEdgeQueryWithTargets(opts *edgeQueryBenchmarkOptions, query *EdgeQu
 	const maxTargetsPerIndex = 100
 
 	// Set a specific seed to allow repeatability
+	// Replace with r := rand.New(rand.NewSource(opts.randomSeed)) and pass through.
 	rand.Seed(opts.randomSeed)
 	opts.randomSeed++
 	indexCap := CapFromCenterAngle(randomPoint(), opts.radiusKm)

s2/paddedcell_test.go

@@ -16,7 +16,6 @@ package s2
 
 import (
 	"math"
-	"math/rand"
 	"testing"
 
 	"github.com/golang/geo/r1"
@@ -133,9 +132,8 @@ func TestPaddedCellEntryExitVertices(t *testing.T) {
 }
 
 func TestPaddedCellShrinkToFit(t *testing.T) {
-	// About 0.2% flaky with a random seed.
-	// TODO: https://github.com/golang/geo/issues/120
-	rand.Seed(1)
+	// TODO: Verify if it's still about 0.2% flaky with a random seed.
+	// TODO(rsned): https://github.com/golang/geo/issues/120
 
 	for iter := 0; iter < 1000; iter++ {
 		// Start with the desired result and work backwards.
@@ -146,7 +144,7 @@ func TestPaddedCellShrinkToFit(t *testing.T) {
 		// Find the biggest rectangle that fits in "result" after padding.
 		// (These calculations ignore numerical errors.)
 		maxPadding := 0.5 * math.Min(sizeUV.X, sizeUV.Y)
-		padding := maxPadding * randomFloat64()
+		padding := randomUniformFloat64(0, maxPadding)
 		maxRect := resultUV.ExpandedByMargin(-padding)
 
 		// Start with a random subset of the maximum rectangle.

s2/point_test.go

@@ -16,7 +16,6 @@ package s2
 
 import (
 	"math"
-	"math/rand"
 	"testing"
 
 	"github.com/golang/geo/r3"
@@ -95,11 +94,10 @@ func TestPointDistance(t *testing.T) {
 }
 
 func TestChordAngleBetweenPoints(t *testing.T) {
-	// About 0.2% flaky with a random seed.
-	// TODO: https://github.com/golang/geo/issues/120
-	rand.Seed(1)
+	// TODO: Verify if it's still about 0.2% flaky with a random seed.
+	// TODO(rsned): https://github.com/golang/geo/issues/120
 
-	for iter := 0; iter < 10; iter++ {
+	for iter := 0; iter < 100; iter++ {
 		m := randomFrame()
 		x := m.col(0)
 		y := m.col(1)

s2/point_vector_test.go

@@ -15,7 +15,6 @@
 package s2
 
 import (
-	"math/rand"
 	"testing"
 )
 
@@ -43,9 +42,6 @@ func TestPointVectorEmpty(t *testing.T) {
 }
 
 func TestPointVectorBasics(t *testing.T) {
-	const seed = 8675309
-	rand.Seed(seed)
-
 	const numPoints = 100
 	var p PointVector = make([]Point, numPoints)
 
@@ -70,7 +66,6 @@ func TestPointVectorBasics(t *testing.T) {
 		t.Errorf("shape.IsFull() = true, want false")
 	}
 
-	rand.Seed(seed)
 	for i := 0; i < numPoints; i++ {
 		if got, want := shape.Chain(i).Start, i; got != want {
 			t.Errorf("shape.Chain(%d).Start = %d, want %d", i, got, want)
@@ -79,7 +74,7 @@ func TestPointVectorBasics(t *testing.T) {
 			t.Errorf("shape.Chain(%d).Length = %v, want %d", i, got, want)
 		}
 		edge := shape.Edge(i)
-		pt := randomPoint()
+		pt := p[i]
 
 		if !pt.ApproxEqual(edge.V0) {
 			t.Errorf("shape.Edge(%d).V0 = %v, want %v", i, edge.V0, pt)
@@ -89,3 +84,7 @@ func TestPointVectorBasics(t *testing.T) {
 		}
 	}
 }
+
+// TODO(rsned): Differences from C++
+// TEST(S2PointVectorShape, ChainIteratorWorks) {
+// TEST(S2PointVectorShape, ChainVertexIteratorWorks) {

s2/rect.go

@@ -31,6 +31,7 @@ type Rect struct {
 }
 
 var (
+	// TODO(rsned): Make these public to match FullLat/FullLng from C++
 	validRectLatRange = r1.Interval{Lo: -math.Pi / 2, Hi: math.Pi / 2}
 	validRectLngRange = s1.FullInterval()
 )

s2/rect_test.go

@@ -1204,5 +1204,5 @@ func TestRectCentroidFullRange(t *testing.T) {
 	// the centroids of the pieces add to give the centroid of their parent.
 	// To make the code simpler we avoid rectangles that cross the 180 degree
 	// line of longitude.
-	testRectCentroidSplitting(t, Rect{r1.Interval{Lo: -math.Pi / 2, Hi: math.Pi / 2}, s1.Interval{Lo: -math.Pi, Hi: math.Pi}}, 10)
+	testRectCentroidSplitting(t, Rect{validRectLatRange, s1.Interval{Lo: -3.14, Hi: 3.14}}, 10)
 }