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

feat: Support customize derivedDataPath #287

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hoangatuan
Copy link
Contributor

Fix issue: #286

@ZevEisenberg
Copy link
Collaborator

Thanks! Is there any way to add a unit tests for this option to make sure it stays working?

@hoangatuan
Copy link
Contributor Author

@ZevEisenberg

I added a unit test.
Also may I ask why the Bitrise CI is falling?

Copy link
Collaborator

@ZevEisenberg ZevEisenberg left a comment

Choose a reason for hiding this comment

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

I don't know if I'm the best one to know if this is the right fix (@rakaramos u up?), but the change looks nice!

@@ -137,6 +137,26 @@ final class BuildForTestingTests: MuterTestCase {
.literal(reason: "Could not parse xctestrun at path: some/project.xctestrun")
)
}

func test_givenTestArgumentsContainsDerivedDataPath_shouldCopyXCTestrunCorrectly() async throws {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo, but also, is this what you want to call it? Or is it more like test_supportsCustomDerivedDataPath or something?

Suggested change
func test_givenTestArgumentsContainsDerivedDataPath_shouldCopyXCTestrunCorrectly() async throws {
func test_givenTestArgumentsContainsDerivedDataPath_shouldCopyXCTestRunCorrectly() async throws {

state.mutatedProjectDirectoryURL = URL(fileURLWithPath: "/path/to/temp")
process.stdoutToBeReturned = xcodebuildBuildForTestingOutput()

_ = try? await sut.run(with: state)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should be able to do this, because the test function is marked throws, unless you expect it to actually throw an error while testing? In that case, I'd probably prefer XCTAssertThrows or an explicit do/catch or something.

Suggested change
_ = try? await sut.run(with: state)
_ = try await sut.run(with: state)

Copy link
Contributor Author

@hoangatuan hoangatuan Dec 18, 2024

Choose a reason for hiding this comment

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

I only provided enough mock data to make the run func execute the copy xctestrun logic, so the run function will eventually throw.
Since I only care about the copy logic getting executed => I used try? here.

@ZevEisenberg
Copy link
Collaborator

No idea why CI is failing. Probably just because we haven't debugged it in a while, and something has slipped out of date. There hasn't been a lot of active development around here lately, unfortunately.

@rakaramos
Copy link
Member

@hoangatuan It turns out this was one of the necessary fixes to get the CI working.

I'm not sure when Xcode changed this behavior, but previously, we could retrieve the build folder by querying xcodebuild with showBuildSettings. Now, however, it only returns SRCROOT + /build, which breaks everything.

That said, you're more than welcome to review #289.
Thanks!

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.

3 participants