Skip to content

Conversation

@zamderax
Copy link
Contributor

@zamderax zamderax commented Oct 10, 2025

  • Convert RoleTests to Swift Testing (@Suite/@test + #expect)
  • Keep all test logic identical; no production code changes
  • Bump swift-tools-version to 6.0 to use built-in Testing

Note: This depends on #33 (Swift 6 CI). CI will pass after #33, and we can migrate remaining tests in small follow-ups.

@hsluoyz hsluoyz closed this Oct 10, 2025
@hsluoyz hsluoyz reopened this Oct 10, 2025
@hsluoyz
Copy link
Member

hsluoyz commented Oct 10, 2025

@zamderax plz fix CI error:

image

@zamderax
Copy link
Contributor Author

Pushed a small fix to address the CI crash: when the effect stream is created with cap == 0, DefaultEffector previously asserted and crashed under Swift 6 + TSAN. I changed it to return a completed stream with res=false instead of asserting.\n\nThis PR remains tests-only in spirit; the change is a minimal safety improvement to prevent crashes in empty-policy cases. If you prefer this fix in a separate PR, I can split it out and point this PR to depend on it.

@hsluoyz
Copy link
Member

hsluoyz commented Oct 10, 2025

@zamderax still not fixed

…currency checks; keep RoleTests using Swift Testing (builds under Swift 6 toolchain)
@zamderax
Copy link
Contributor Author

zamderax commented Oct 10, 2025

Ah I see I'll address the CI failures by reverting swift-tools-version to 5.3 in this tests-only PR. The GitHub Action is already running on Swift 6 (see #33), so the Swift Testing module is available to the toolchain even with a 5.3 manifest, and RoleTests compiles and runs.

This avoids triggering Swift 6 strict concurrency checks across the legacy codebase in the same PR. We can migrate production code for full Swift 6 sendability in separate, focused PRs.

Also included a small safety fix to DefaultEffector for cap == 0 (prevents TSAN crash).

@hsluoyz hsluoyz changed the title Tests: migrate RoleManager tests to Swift Testing (phase 1) feat(tests): migrate RoleManager tests to Swift Testing (phase 1) Oct 10, 2025
@hsluoyz hsluoyz merged commit 8ec9705 into casbin:master Oct 10, 2025
4 checks passed
@hsluoyz
Copy link
Member

hsluoyz commented Oct 10, 2025

🎉 This PR is included in version 1.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants