test: add test to assert empty intersection invalid

[adriantam]

Description

What problem is being solved?

Ensure weighted graph prevents case where model has relation with empty intersection / union as it can cause check / list objects / list users algorithm to fail. We had seen problems with OpenFGA's type system. We want to ensure this will not be a regression when we switch to use weighted graph as replacement for typesystem.

How is it being solved?

Add unit test to assert these models are marked as invalid.

What changes are made to solve it?

References

openfga/openfga#2865

Review Checklist

• I have clicked on "allow edits by maintainers".
• I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
• The correct base branch is being used, if not main
• I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

• Tests
  • Expanded test coverage for error scenarios in graph construction, validating proper handling of invalid configurations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Added test cases validating that the weighted graph builder properly rejects invalid graph models where intersection or union nodes contain no children. Test file enhanced with new subtests under an existing test function alongside necessary import alias for OpenFGA v1 API types.

Changes

Cohort / File(s)	Summary
Test Coverage for Invalid Graph Models pkg/go/graph/weighted_graph_builder_test.go	Added empty_intersection and empty_union subtests to validate graph construction failure scenarios; introduced openfgav1 import alias for API type references

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title partially addresses the changeset, focusing only on empty intersection when the PR also adds tests for empty union cases.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dosubot]

Related Documentation

Checked 7 published document(s) in 1 knowledge base(s). No updates required.

^{How did I do? Any feedback?}  

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

pkg/go/graph/weighted_graph_builder_test.go (2)

1939-1969: Use more specific error assertion.

The test currently uses require.Error(t, err) which only checks that some error occurred. For consistency with similar tests in this file (see lines 408, 1917, 1937), consider using a more specific assertion to verify the expected error type or message.

♻️ Suggested improvement

If the builder returns ErrInvalidModel for empty intersections:

 		wgb := NewWeightedAuthorizationModelGraphBuilder()
 		_, err := wgb.Build(authorizationModel)
-		require.Error(t, err)
+		require.ErrorIs(t, err, ErrInvalidModel)

Alternatively, if there's a specific error message:

 		wgb := NewWeightedAuthorizationModelGraphBuilder()
 		_, err := wgb.Build(authorizationModel)
-		require.Error(t, err)
+		require.ErrorContains(t, err, "empty intersection")

---

1970-2000: Use more specific error assertion.

Similar to the empty_intersection test, this test uses require.Error(t, err) which only checks that some error occurred. Consider using a more specific assertion for consistency with other tests in this file.

♻️ Suggested improvement

If the builder returns ErrInvalidModel for empty unions:

 		wgb := NewWeightedAuthorizationModelGraphBuilder()
 		_, err := wgb.Build(authorizationModel)
-		require.Error(t, err)
+		require.ErrorIs(t, err, ErrInvalidModel)

Alternatively, if there's a specific error message:

 		wgb := NewWeightedAuthorizationModelGraphBuilder()
 		_, err := wgb.Build(authorizationModel)
-		require.Error(t, err)
+		require.ErrorContains(t, err, "empty union")

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 490a32d and 2193c79.

📒 Files selected for processing (1)

• pkg/go/graph/weighted_graph_builder_test.go

🧰 Additional context used

🧬 Code graph analysis (1)

pkg/go/graph/weighted_graph_builder_test.go (1)

pkg/go/graph/weighted_graph_builder.go (1)

• NewWeightedAuthorizationModelGraphBuilder (19-21)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (1)

pkg/go/graph/weighted_graph_builder_test.go (1)

6-6: LGTM! Import is necessary for the new test cases.

The import alias is correctly added to support direct construction of authorization models using proto types in the new tests.