Skip to content

Commit 4fc7de4

Browse files
barakmichbarakmich
committed
fix: longer timeout and race condition fix
1 parent 5ace6b9 commit 4fc7de4

File tree

8 files changed

+108
-50
lines changed

8 files changed

+108
-50
lines changed

‎internal/services/integrationtesting/query_plan_consistency_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,20 +78,23 @@ func runQueryPlanConsistencyForFile(t *testing.T, filePath string) {
7878

7979
t.Run("lookup_resources", func(t *testing.T) {
8080
if os.Getenv("TEST_QUERY_PLAN_RESOURCES") == "" {
81-
t.Skip("Skipping IterResources tests: set TEST_QUERY_PLAN_RESOURCES=true to enable")
81+
t.Skip("Skipping IterResources tests due to deprectation: Set TEST_QUERY_PLAN_RESOURCES=true to enable")
8282
}
8383
runQueryPlanLookupResources(t, handle)
8484
})
8585

8686
t.Run("lookup_subjects", func(t *testing.T) {
8787
if os.Getenv("TEST_QUERY_PLAN_SUBJECTS") == "" {
88-
t.Skip("Skipping IterSubjects tests: set TEST_QUERY_PLAN_SUBJECTS=true to enable")
88+
t.Skip("Skipping IterSubjects tests due to deprectation: Set TEST_QUERY_PLAN_SUBJECTS=true to enable")
8989
}
9090
runQueryPlanLookupSubjects(t, handle)
9191
})
9292
}
9393

9494
func runQueryPlanAssertions(t *testing.T, handle *queryPlanConsistencyHandle) {
95+
if os.Getenv("TEST_QUERY_PLAN_CHECK") == "" {
96+
t.Skip("Skipping Check tests due to deprecation. Set TEST_QUERY_PLAN_CHECK=true to enable")
97+
}
9598
t.Run("assertions", func(t *testing.T) {
9699
for _, parsedFile := range handle.populated.ParsedFiles {
97100
for _, entry := range []struct {

‎magefiles/test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (Test) Image(ctx context.Context) error {
6060
// Integration Run integration tests
6161
func (Test) Integration(ctx context.Context) error {
6262
mg.Deps(checkDocker)
63-
if err := goTest(ctx, "./internal/services/integrationtesting/...", "-tags", "ci,docker", "-timeout", "15m"); err != nil {
63+
if err := goTest(ctx, "./internal/services/integrationtesting/...", "-tags", "ci,docker", "-timeout", "20m"); err != nil {
6464
return err
6565
}
6666

@@ -100,7 +100,7 @@ func (Test) E2e(ctx context.Context, crdbVersion string) error {
100100
// IntegrationCover Run integration tests with cover
101101
func (Test) IntegrationCover(ctx context.Context) error {
102102
mg.Deps(checkDocker)
103-
args := []string{"-tags", "ci,docker", "-timeout", "15m", "-count=1"}
103+
args := []string{"-tags", "ci,docker", "-timeout", "20m", "-count=1"}
104104
args = append(args, coverageFlags...)
105105
return goTest(ctx, "./internal/services/integrationtesting/...", args...)
106106
}

‎pkg/query/alias_test.go‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,11 @@ import (
99
func TestAliasIterator(t *testing.T) {
1010
t.Parallel()
1111

12-
// Create test context
13-
ctx := NewLocalContext(t.Context())
14-
1512
t.Run("Check_BasicRelationRewriting", func(t *testing.T) {
1613
t.Parallel()
1714
require := require.New(t)
1815

16+
ctx := NewLocalContext(t.Context())
1917
// Create a sub-iterator with document relations
2018
subIt := NewDocumentAccessFixedIterator()
2119

@@ -47,6 +45,7 @@ func TestAliasIterator(t *testing.T) {
4745
t.Parallel()
4846
require := require.New(t)
4947

48+
ctx := NewLocalContext(t.Context())
5049
// Create an empty sub-iterator since we only want to test self-edge detection
5150
subIt := NewEmptyFixedIterator()
5251

@@ -79,6 +78,7 @@ func TestAliasIterator(t *testing.T) {
7978
t.Parallel()
8079
require := require.New(t)
8180

81+
ctx := NewLocalContext(t.Context())
8282
// Create a sub-iterator
8383
subIt := NewSingleUserFixedIterator("bob")
8484

@@ -103,6 +103,7 @@ func TestAliasIterator(t *testing.T) {
103103
t.Parallel()
104104
require := require.New(t)
105105

106+
ctx := NewLocalContext(t.Context())
106107
subIt := NewDocumentAccessFixedIterator()
107108
aliasIt := NewAliasIterator("access", subIt)
108109

@@ -130,6 +131,7 @@ func TestAliasIterator(t *testing.T) {
130131
t.Parallel()
131132
require := require.New(t)
132133

134+
ctx := NewLocalContext(t.Context())
133135
subIt := NewDocumentAccessFixedIterator()
134136
aliasIt := NewAliasIterator("permission", subIt)
135137

@@ -151,6 +153,7 @@ func TestAliasIterator(t *testing.T) {
151153
t.Parallel()
152154
require := require.New(t)
153155

156+
ctx := NewLocalContext(t.Context())
154157
subIt := NewDocumentAccessFixedIterator()
155158
aliasIt := NewAliasIterator("can_view", subIt)
156159

@@ -170,6 +173,7 @@ func TestAliasIterator(t *testing.T) {
170173
t.Parallel()
171174
require := require.New(t)
172175

176+
ctx := NewLocalContext(t.Context())
173177
// Create an empty sub-iterator since we only want to test self-edge detection
174178
subIt := NewEmptyFixedIterator()
175179

@@ -202,6 +206,7 @@ func TestAliasIterator(t *testing.T) {
202206
t.Parallel()
203207
require := require.New(t)
204208

209+
ctx := NewLocalContext(t.Context())
205210
subIt := NewEmptyFixedIterator()
206211
aliasIt := NewAliasIterator("empty_alias", subIt)
207212

@@ -217,6 +222,7 @@ func TestAliasIterator(t *testing.T) {
217222
t.Parallel()
218223
require := require.New(t)
219224

225+
ctx := NewLocalContext(t.Context())
220226
// Create empty sub-iterator
221227
subIt := NewEmptyFixedIterator()
222228
aliasIt := NewAliasIterator("self", subIt)
@@ -238,6 +244,7 @@ func TestAliasIterator(t *testing.T) {
238244
t.Parallel()
239245
require := require.New(t)
240246

247+
ctx := NewLocalContext(t.Context())
241248
subIt := NewEmptyFixedIterator()
242249
aliasIt := NewAliasIterator("owner", subIt)
243250

@@ -320,13 +327,11 @@ func TestAliasIteratorExplain(t *testing.T) {
320327
func TestAliasIteratorErrorHandling(t *testing.T) {
321328
t.Parallel()
322329

323-
// Create test context
324-
ctx := NewLocalContext(t.Context())
325-
326330
t.Run("Check_SubIteratorError", func(t *testing.T) {
327331
t.Parallel()
328332
require := require.New(t)
329333

334+
ctx := NewLocalContext(t.Context())
330335
// Create a faulty sub-iterator
331336
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
332337
aliasIt := NewAliasIterator("error_test", faultyIt)
@@ -339,6 +344,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
339344
t.Parallel()
340345
require := require.New(t)
341346

347+
ctx := NewLocalContext(t.Context())
342348
// Create a faulty sub-iterator that fails during collection
343349
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
344350
aliasIt := NewAliasIterator("collection_error_test", faultyIt)
@@ -355,6 +361,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
355361
t.Parallel()
356362
require := require.New(t)
357363

364+
ctx := NewLocalContext(t.Context())
358365
// Create a faulty sub-iterator that fails on IterSubjectsImpl
359366
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
360367
aliasIt := NewAliasIterator("error_test", faultyIt)
@@ -368,6 +375,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
368375
t.Parallel()
369376
require := require.New(t)
370377

378+
ctx := NewLocalContext(t.Context())
371379
// Create a faulty sub-iterator that fails during collection
372380
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
373381
aliasIt := NewAliasIterator("collection_error_test", faultyIt)
@@ -385,6 +393,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
385393
t.Parallel()
386394
require := require.New(t)
387395

396+
ctx := NewLocalContext(t.Context())
388397
// Create a faulty sub-iterator that fails on IterResourcesImpl
389398
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
390399
aliasIt := NewAliasIterator("error_test", faultyIt)
@@ -398,6 +407,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
398407
t.Parallel()
399408
require := require.New(t)
400409

410+
ctx := NewLocalContext(t.Context())
401411
// Create a faulty sub-iterator that fails during collection
402412
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
403413
aliasIt := NewAliasIterator("collection_error_test", faultyIt)
@@ -415,6 +425,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
415425
t.Parallel()
416426
require := require.New(t)
417427

428+
ctx := NewLocalContext(t.Context())
418429
// Create a faulty sub-iterator that errors on CheckImpl
419430
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
420431
aliasIt := NewAliasIterator("self", faultyIt)
@@ -431,6 +442,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
431442
t.Parallel()
432443
require := require.New(t)
433444

445+
ctx := NewLocalContext(t.Context())
434446
// Create a faulty sub-iterator that fails during collection
435447
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
436448
aliasIt := NewAliasIterator("self", faultyIt)
@@ -450,13 +462,11 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
450462
func TestAliasIteratorAdvancedScenarios(t *testing.T) {
451463
t.Parallel()
452464

453-
// Create test context
454-
ctx := NewLocalContext(t.Context())
455-
456465
t.Run("Check_MultipleResourcesSelfEdgeWithSubResults", func(t *testing.T) {
457466
t.Parallel()
458467
require := require.New(t)
459468

469+
ctx := NewLocalContext(t.Context())
460470
// Create sub-iterator with real data
461471
subIt := NewDocumentAccessFixedIterator()
462472
aliasIt := NewAliasIterator("admin", subIt)
@@ -497,6 +507,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
497507
t.Parallel()
498508
require := require.New(t)
499509

510+
ctx := NewLocalContext(t.Context())
500511
// Create empty sub-iterator to isolate self-edge logic
501512
subIt := NewEmptyFixedIterator()
502513
aliasIt := NewAliasIterator("owner", subIt)
@@ -525,6 +536,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
525536
t.Parallel()
526537
require := require.New(t)
527538

539+
ctx := NewLocalContext(t.Context())
528540
subIt := NewEmptyFixedIterator()
529541
aliasIt := NewAliasIterator("empty_relation", subIt)
530542

@@ -540,6 +552,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
540552
t.Parallel()
541553
require := require.New(t)
542554

555+
ctx := NewLocalContext(t.Context())
543556
subIt := NewEmptyFixedIterator()
544557
aliasIt := NewAliasIterator("empty_relation", subIt)
545558

@@ -555,6 +568,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
555568
t.Parallel()
556569
require := require.New(t)
557570

571+
ctx := NewLocalContext(t.Context())
558572
// Use iterator with many results
559573
subIt := NewLargeFixedIterator()
560574
aliasIt := NewAliasIterator("massive", subIt)
@@ -579,6 +593,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
579593
t.Parallel()
580594
require := require.New(t)
581595

596+
ctx := NewLocalContext(t.Context())
582597
subIt := NewDocumentAccessFixedIterator()
583598
original := NewAliasIterator("original", subIt)
584599

@@ -615,6 +630,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
615630
t.Parallel()
616631
require := require.New(t)
617632

633+
ctx := NewLocalContext(t.Context())
618634
subIt := NewDocumentAccessFixedIterator()
619635
aliasIt := NewAliasIterator("early_test", subIt)
620636

@@ -638,6 +654,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
638654
t.Parallel()
639655
require := require.New(t)
640656

657+
ctx := NewLocalContext(t.Context())
641658
subIt := NewDocumentAccessFixedIterator()
642659
aliasIt := NewAliasIterator("admin", subIt)
643660

@@ -663,6 +680,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
663680
t.Parallel()
664681
require := require.New(t)
665682

683+
ctx := NewLocalContext(t.Context())
666684
subIt := NewDocumentAccessFixedIterator()
667685
aliasIt := NewAliasIterator("early_subjects", subIt)
668686

@@ -686,6 +704,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
686704
t.Parallel()
687705
require := require.New(t)
688706

707+
ctx := NewLocalContext(t.Context())
689708
subIt := NewDocumentAccessFixedIterator()
690709
aliasIt := NewAliasIterator("early_resources", subIt)
691710

@@ -709,6 +728,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
709728
t.Parallel()
710729
require := require.New(t)
711730

731+
ctx := NewLocalContext(t.Context())
712732
// Use sub-iterator that will return paths that need rewriting
713733
subIt := NewDocumentAccessFixedIterator()
714734
aliasIt := NewAliasIterator("admin", subIt)

‎pkg/query/context.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package query
22

33
import (
44
"context"
5+
"sync"
56

67
"github.com/authzed/spicedb/internal/caveats"
78
"github.com/authzed/spicedb/pkg/datalayer"
@@ -39,6 +40,7 @@ type Context struct {
3940
// IterSubjects call on this context. Nil means not yet set.
4041
// Used to wrap only the top-level call with DeduplicatePathSeq.
4142
topLevelIterator Iterator
43+
topLevelOnce sync.Once
4244
}
4345

4446
// NewLocalContext creates a new query execution context with a LocalExecutor.
@@ -243,10 +245,11 @@ func (ctx *Context) IterSubjects(it Iterator, resource Object, filterSubjectType
243245
return nil, spiceerrors.MustBugf("no executor has been set")
244246
}
245247

246-
isTopLevel := ctx.topLevelIterator == nil
247-
if isTopLevel {
248+
isTopLevel := false
249+
ctx.topLevelOnce.Do(func() {
248250
ctx.topLevelIterator = it
249-
}
251+
isTopLevel = true
252+
})
250253

251254
tracedIterator := ctx.traceEnterIfEnabled(it, iterSubjectsTraceString(resource, filterSubjectType))
252255
key := it.CanonicalKey()
@@ -275,10 +278,11 @@ func (ctx *Context) IterResources(it Iterator, subject ObjectAndRelation, filter
275278
return nil, spiceerrors.MustBugf("no executor has been set")
276279
}
277280

278-
isTopLevel := ctx.topLevelIterator == nil
279-
if isTopLevel {
281+
isTopLevel := false
282+
ctx.topLevelOnce.Do(func() {
280283
ctx.topLevelIterator = it
281-
}
284+
isTopLevel = true
285+
})
282286

283287
tracedIterator := ctx.traceEnterIfEnabled(it, iterResourcesTraceString(subject, filterResourceType))
284288
key := it.CanonicalKey()

0 commit comments

Comments
 (0)