Repository: amp-labs/connectors
Pull request #1575 (2025-04-19 00:13:32)

feat(test): Soft equals NextPage URLs
# Problem
Strict string comparison of `NextPage` tokens in mock tests isn't reliable:
```
actual.NextPage.String() == expectedNextPage
```
Query parameters can appear in different orders, making equivalent URLs fail the comparison.

# Fix
Introduced a `compareNextPageToken` method used by all read test suites. It delegates to the `urlbuilder.URL` equality check.
Added test cases with semantically equivalent but differently ordered URLs to validate the implementation.

Cobalt0s: > [!WARNING]
> <b>This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1575?utm_source=stack-comment-downstack-mergeability-warning" >on Graphite</a>.</b>
> <a href="https://graphite.dev/docs/merge-pull-requests">Learn more</a>

* **#1579** <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1579?utm_source=stack-comment-icon" target="_blank"><img src="https://static.graphite.dev/graphite-32x32-black.png" alt="Graphite" width="10px" height="10px"/></a>
* **#1578** <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1578?utm_source=stack-comment-icon" target="_blank"><img src="https://static.graphite.dev/graphite-32x32-black.png" alt="Graphite" width="10px" height="10px"/></a>
* **#1577** <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1577?utm_source=stack-comment-icon" target="_blank"><img src="https://static.graphite.dev/graphite-32x32-black.png" alt="Graphite" width="10px" height="10px"/></a>
* **#1576** <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1576?utm_source=stack-comment-icon" target="_blank"><img src="https://static.graphite.dev/graphite-32x32-black.png" alt="Graphite" width="10px" height="10px"/></a>
* **#1575** <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1575?utm_source=stack-comment-icon" target="_blank"><img src="https://static.graphite.dev/graphite-32x32-black.png" alt="Graphite" width="10px" height="10px"/></a> 👈 <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1575?utm_source=stack-comment-view-in-graphite" target="_blank">(View in Graphite)</a>
* **#1560** <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/1560?utm_source=stack-comment-icon" target="_blank"><img src="https://static.graphite.dev/graphite-32x32-black.png" alt="Graphite" width="10px" height="10px"/></a>
* `main`


<h2></h2>

This stack of pull requests is managed by <a href="https://graphite.dev?utm-source=stack-comment"><b>Graphite</b></a>. Learn more about <a href="https://stacking.dev/?utm_source=stack-comment">stacking</a>.
<!-- Current dependencies on/for this PR: -->

Patch:
diff --git a/common/urlbuilder/url.go b/common/urlbuilder/url.go
index f5492920..e4c4504e 100644
--- a/common/urlbuilder/url.go
+++ b/common/urlbuilder/url.go
@@ -4,6 +4,8 @@ import (
 	"errors"
 	"net/url"
 	"strings"
+
+	"github.com/amp-labs/connectors/internal/datautils"
 )
 
 var ErrInvalidURL = errors.New("URL format is incorrect")
@@ -152,3 +154,29 @@ func cleanTrailingSlashes(link string) string {
 func (u *URL) HasQueryParam(name string) bool {
 	return u.queryParams.Has(name)
 }
+
+// Equals compares URL equality ignoring order, encoding.
+func (u *URL) Equals(other *URL) bool { // nolint:cyclop
+	if strings.ToLower(u.delegate.Host) != strings.ToLower(other.delegate.Host) || // nolint:staticcheck
+		u.delegate.Path != other.delegate.Path ||
+		u.delegate.RawPath != other.delegate.RawPath ||
+		u.delegate.Scheme != other.delegate.Scheme ||
+		u.delegate.Fragment != other.delegate.Fragment ||
+		u.delegate.RawFragment != other.delegate.RawFragment {
+		return false
+	}
+
+	// Compare query parameters. The order doesn't matter
+	if len(u.queryParams) != len(other.queryParams) {
+		return false
+	}
+
+	for name, params := range u.queryParams {
+		otherParams, exists := other.queryParams[name]
+		if !exists || !datautils.EqualUnordered(params, otherParams) {
+			return false
+		}
+	}
+
+	return true
+}
diff --git a/internal/datautils/slice.go b/internal/datautils/slice.go
new file mode 100644
index 00000000..b6197307
--- /dev/null
+++ b/internal/datautils/slice.go
@@ -0,0 +1,21 @@
+package datautils
+
+func EqualUnordered[T comparable](a, b []T) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	count := make(map[T]int)
+	for _, v := range a {
+		count[v]++
+	}
+
+	for _, v := range b {
+		count[v]--
+		if count[v] < 0 {
+			return false
+		}
+	}
+
+	return true
+}


Test patch:
diff --git a/common/urlbuilder/url_test.go b/common/urlbuilder/url_test.go
index e8f60a1d..f70280d4 100644
--- a/common/urlbuilder/url_test.go
+++ b/common/urlbuilder/url_test.go
@@ -254,3 +254,113 @@ func TestFromRawURL(t *testing.T) { // nolint:funlen
 		})
 	}
 }
+
+func TestEquality(t *testing.T) { // nolint:funlen
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		a     string
+		b     string
+		equal bool
+	}{
+		{
+			name:  "Query params in different order",
+			a:     "https://example.com/path?a=1&b=2",
+			b:     "https://example.com/path?b=2&a=1",
+			equal: true,
+		},
+		{
+			name:  "Repeated query params, same values different order",
+			a:     "https://example.com/path?a=1&a=2",
+			b:     "https://example.com/path?a=2&a=1",
+			equal: true,
+		},
+		{
+			name:  "Encoded vs unencoded space in query",
+			a:     "https://example.com/path?q=hello+world",
+			b:     "https://example.com/path?q=hello%20world",
+			equal: true,
+		},
+		{
+			name:  "Empty query vs missing query",
+			a:     "https://example.com/path?",
+			b:     "https://example.com/path",
+			equal: true,
+		},
+		{
+			name:  "Mixed order and encoding",
+			a:     "https://example.com/path?x=1&y=a%2Fb",
+			b:     "https://example.com/path?y=a%2Fb&x=1",
+			equal: true,
+		},
+		{
+			name:  "Case-insensitive host",
+			a:     "https://EXAMPLE.com/path?a=1",
+			b:     "https://example.com/path?a=1",
+			equal: true,
+		},
+		{
+			name:  "No query vs query with just question mark",
+			a:     "https://example.com/path",
+			b:     "https://example.com/path?",
+			equal: true,
+		},
+		{
+			name:  "Same key repeated multiple times in different order",
+			a:     "https://example.com/path?x=1&x=2",
+			b:     "https://example.com/path?x=2&x=1",
+			equal: true,
+		},
+		{
+			name:  "Same URL, different query param order, same fragment",
+			a:     "https://example.com/path?a=1&b=2#section1",
+			b:     "https://example.com/path?b=2&a=1#section1",
+			equal: true,
+		},
+		{
+			name:  "Symbol encoding",
+			a:     "https://example.com/reset?email=user@example.com",
+			b:     "https://example.com/reset?email=user%40example.com",
+			equal: true,
+		},
+		{
+			name:  "Different origin",
+			a:     "https://example.com",
+			b:     "https://canada.gov",
+			equal: false,
+		},
+		{
+			name:  "Different paths",
+			a:     "https://example.com/customers",
+			b:     "https://example.com/orders",
+			equal: false,
+		},
+		{
+			name:  "Different query param values",
+			a:     "https://example.com/orders?customer=Bob",
+			b:     "https://example.com/orders?customer=Alice",
+			equal: false,
+		},
+	}
+
+	for _, tt := range tests { // nolint:varnamelen
+		// nolint:varnamelen
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			firstURL, err := New(tt.a)
+			if err != nil {
+				t.Fatalf("%s: is an invalid test, check urls", tt.name)
+			}
+
+			secondURL, err := New(tt.b)
+			if err != nil {
+				t.Fatalf("%s: is an invalid test, check urls", tt.name)
+			}
+
+			output := firstURL.Equals(secondURL)
+			testutils.CheckOutput(t, tt.name, tt.equal, output)
+		})
+	}
+}
diff --git a/test/utils/testroutines/comparator.go b/test/utils/testroutines/comparator.go
index ffd088d2..f95aeeef 100644
--- a/test/utils/testroutines/comparator.go
+++ b/test/utils/testroutines/comparator.go
@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	"github.com/amp-labs/connectors/common"
+	"github.com/amp-labs/connectors/common/urlbuilder"
 	"github.com/amp-labs/connectors/test/utils/mockutils"
 )
 
@@ -33,14 +34,42 @@ func ComparatorSubsetRead(serverURL string, actual, expected *common.ReadResult)
 //	 }
 //
 // At runtime this may look as follows: http://127.0.0.1:38653/v3/contacts?cursor=bGltaXQ9MSZuZXh0PTI=.
+// The query parameters in URL can be in different order, encoding could differ as soon as the URL content matches
+// the check will conclude that pagination matches.
 func ComparatorPagination(serverURL string, actual *common.ReadResult, expected *common.ReadResult) bool {
 	expectedNextPage := resolveTestServerURL(expected.NextPage.String(), serverURL)
 
-	return actual.NextPage.String() == expectedNextPage &&
+	return compareNextPageToken(actual.NextPage.String(), expectedNextPage) &&
 		actual.Rows == expected.Rows &&
 		actual.Done == expected.Done
 }
 
+func compareNextPageToken(actual, expected string) bool {
+	if len(actual) == 0 && len(expected) == 0 {
+		return true
+	}
+
+	if !strings.HasPrefix(actual, "http") {
+		// Next page token is not a URL, compare raw text.
+		return actual == expected
+	}
+
+	// We are dealing with URLs.
+	// Compare URLs ignoring the query parameter order or encoding.
+	// However, the "data content" must match.
+	actualURL, err := urlbuilder.New(actual)
+	if err != nil {
+		return false
+	}
+
+	expectedURL, err := urlbuilder.New(expected)
+	if err != nil {
+		return false
+	}
+
+	return actualURL.Equals(expectedURL)
+}
+
 // ComparatorSubsetWrite ensures that only the specified metadata objects are present,
 // while other values are verified through an exact match..
 func ComparatorSubsetWrite(_ string, actual, expected *common.WriteResult) bool {