From 731ec86af45752bf9cd3605ebb4d513cc7b8689b Mon Sep 17 00:00:00 2001
From: David Ackroyd <23301187+dackroyd@users.noreply.github.com>
Date: Wed, 3 Apr 2024 23:11:56 +1100
Subject: [PATCH] Fix Flaky Order of Directive Validation

Due to the use of a map behind the directiveNames, iterating over them is done
in random order. When there are multiple directives that are duplicated, the
order of errors can change between runs, and be inconsistent with the
expecated result.

This issue is also the case for other types which use a map to validate names,
however the other cases currently have no tests validating this condition. To
resolve the immediate issue of flaky tests, addressing just the directives case
for now. Resolution of other cases should be accompanied by tests exposing the
issue.
---
 internal/validation/validation.go | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/internal/validation/validation.go b/internal/validation/validation.go
index 42fe6e53..3c410f96 100644
--- a/internal/validation/validation.go
+++ b/internal/validation/validation.go
@@ -698,7 +698,15 @@ func validateDirectives(c *opContext, loc string, directives ast.DirectiveList)
 		)
 	}
 
-	for n := range directiveNames {
+	// Iterating in the declared order, rather than using the directiveNames ordering which is random
+	for _, d := range directives {
+		n := d.Name.Name
+
+		ds := directiveNames[n]
+		if len(ds) <= 1 {
+			continue
+		}
+
 		dd, ok := c.schema.Directives[n]
 		if !ok {
 			// Invalid directive will have been flagged already
@@ -709,11 +717,6 @@ func validateDirectives(c *opContext, loc string, directives ast.DirectiveList)
 			continue
 		}
 
-		ds := directiveNames[n]
-		if len(ds) <= 1 {
-			continue
-		}
-
 		for _, loc := range ds[1:] {
 			// Duplicate directive errors are inconsistent with the behaviour for other types in graphql-js
 			// Instead of reporting a single error with all locations, errors are reported for each duplicate after the first declaration
@@ -722,6 +725,9 @@ func validateDirectives(c *opContext, loc string, directives ast.DirectiveList)
 				return fmt.Sprintf("The directive %q can only be used once at this location.", "@"+n)
 			})
 		}
+
+		// drop the name from the set to prevent the same errors being re-added for duplicates
+		delete(directiveNames, n)
 	}
 }