Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override Test ID to ensure trait dependencies are cached #293

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jflan-dd
Copy link

I ran into an issue where reference typed dependencies weren't correctly cached if they were updated in-place rather than being fully overridden.

The root cause of this seems to be that Test.current isn't set during prepare(for test: Test) and as a result the CacheKey.testIdentifier was nil during dependency setup.

This PR creates an internal TestID struct that can be explicitly overridden during dependency prep, but perhaps there's a more elegant solution that could be implemented in xctest-dymaic-overlay directly.

@jflan-dd
Copy link
Author

jflan-dd commented Oct 16, 2024

It seems like this could also be solved with a TestExecuting conformance if https://forums.swift.org/t/pitch-custom-test-execution-traits/75055/14 lands, since Test.current is set before _executeTraits is called

Is there a plan for how to be backwards compatible with different versions of swift-testing as new features are added?

Comment on lines +402 to +403
// unsafeBitCast used because current version of xctest-dynamic-overlay does not expose init
self.testIdentifier = unsafeBitCast(testID, to: TestContext.Testing.Test.ID.self)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hack is the main reason I marked this PR as a draft

@stephencelis
Copy link
Member

@jflan-dd Thanks for looking into this and putting together a PR! We'll try to review it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants