Add simulated location support on testable target configuration #6187

woin2ee · 2024-04-14T12:06:22Z

Resolves #5375

Short description 📝

This PR adds the simulatedLocation property to the TestableTarget.

The API looks like this

testAction: .targets(
    [
        .testableTarget(
            target: "AppTests",
            simulatedLocation: .rioDeJaneiro
//          simulatedLocation: .custom(gpxFile: "Resources/Grand Canyon.gpx")
        ),
    ],
    arguments: .arguments()
)

then...

How to test the changes locally 🧐

Run tuist generate in the ios_app_with_custom_scheme fixture. Then check the Simulated location setting for the test target in the test action of the App-Debug or App-Release scheme of the created project.

Contributor checklist ✅

The code has been linted using run mise run lint:fix
The change is tested via unit testing or acceptance testing, or both
The title of the PR is formulated in a way that is usable as a changelog entry
In case the PR introduces changes that affect users, the documentation has been updated

Reviewer checklist ✅

The code architecture and patterns are consistent with the rest of the codebase
Reviewer has checked that, if needed, the PR includes the label changelog:added, changelog:fixed, or changelog:changed, and the title is usable as a changelog entry

fortmarek

Great work 👏

It would be amazing if we could test the location is added to the test action via an acceptance test. We can add an assertion similar to this that would check the simulated location is added for the test target and you would test that here.

Let me know if you need any help setting that up 🤞

Sources/ProjectDescription/RunActionOptions.swift

fortmarek · 2024-04-16T07:09:04Z

Sources/TuistGenerator/Generator/ProjectFileElements.swift

+        fileElements.formUnion(runActionGPXFiles)
+
+        let testActionGPXFiles = project.schemes.compactMap { scheme -> [GroupFileElement] in
+            guard let testAction = scheme.testAction else { return [] }
+
+            let elements = testAction.targets.compactMap { target -> GroupFileElement? in
+                guard case let .gpxFile(path) = target.simulatedLocation else {
+                    return nil
+                }
+
+                return GroupFileElement(path: path, group: project.filesGroup)
+            }
+
+            return elements
+        }
+        .flatMap { $0 }
+
+        fileElements.formUnion(testActionGPXFiles)


Can we test the logic here in ProjectFileElementsTests?

Added test cases for extracting GPX files.

woin2ee · 2024-04-16T17:12:27Z

Tests/TuistGeneratorAcceptanceTests/GenerateAcceptanceTests.swift

        try await run(BuildCommand.self, "App-Local")
+
+        let xcodeprojPath = fixturePath.appending(components: ["App", "MainApp.xcodeproj"])
+
+        try XCTAssertSimulatedLocationAdded(
+            xcodeprojPath: xcodeprojPath,


For this test case, I specified the path manually because the .xcodeproj file is not generated in the root of the fixture. I would like to ask if this is a good way to do it.

Yeah, that's fine 🙌

woin2ee · 2024-04-16T17:29:41Z

I apologize for my force push. I didn't realize this would happen... 😓

fortmarek

Thanks a lot for adding the acceptance tests ❤️ Just two nits, afterwards, we can merge this one 🙂

fortmarek · 2024-04-19T07:18:16Z

Tests/TuistGeneratorAcceptanceTests/GenerateAcceptanceTests.swift

        try await run(BuildCommand.self, "App-Local")
+
+        let xcodeprojPath = fixturePath.appending(components: ["App", "MainApp.xcodeproj"])
+
+        try XCTAssertSimulatedLocationAdded(
+            xcodeprojPath: xcodeprojPath,


Yeah, that's fine 🙌

fortmarek · 2024-04-19T07:19:43Z

Sources/TuistAcceptanceTesting/TuistAcceptanceTestCase+Extra.swift

+    ///   - testTarget: A specific test target name.
+    ///   - simulatedLocation: A simulated location. This value can be passed a `location string` or a `GPX filename`.
+    ///   For example, "Rio de Janeiro, Brazil" or "Grand Canyon.gpx".
+    public func XCTAssertSimulatedLocationAdded(


nit: Can we rename this to XCTAssertContainsSimulatedLocation? It's more inline with how we name these custom assert methods.

Sure! That makes sense!

fortmarek · 2024-04-19T07:20:15Z

Sources/TuistAcceptanceTesting/TuistAcceptanceTestCase+Extra.swift

+        line: UInt = #line
+    ) throws {
+        if let xcodeprojPath {
+            self.xcodeprojPath = xcodeprojPath


I wouldn't override the self.xcodeprojPath, I would only use the custom one directly in this method.

It's a great way to keep it simple 👍.

fortmarek · 2024-04-22T07:23:31Z

@all-contributors add @woin2ee for code

allcontributors · 2024-04-22T07:23:41Z

@fortmarek

I've put up a pull request to add @woin2ee! 🎉

fortmarek · 2024-04-22T07:24:02Z

Amazing work 👏

woin2ee · 2024-04-22T13:08:14Z

I was able to finish it with your help. Thank you :)

fortmarek reviewed Apr 16, 2024

View reviewed changes

woin2ee commented Apr 16, 2024

View reviewed changes

woin2ee added 6 commits April 17, 2024 02:20

Support simulated location for testable targets

fcf6cb0

Fix typo in default location

7b6dfc1

Add simulated location to ios_app_with_custom_scheme fixture

0783f8a

Add test for finding GPX files

b40c47f

Avoid breaking compatibility

dcfbc81

Add assertions for simulated location

e439ead

woin2ee force-pushed the scheme branch from df32f1c to e439ead Compare April 16, 2024 17:20

fortmarek approved these changes Apr 19, 2024

View reviewed changes

Make assertion method clear

b35dd36

allcontributors bot mentioned this pull request Apr 22, 2024

add woin2ee as a contributor for code #6216

Merged

fortmarek merged commit afcd2dc into tuist:main Apr 22, 2024
8 checks passed

fortmarek added the changelog:added PR will be listed in the Added section of CHANGELOG label Apr 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simulated location support on testable target configuration #6187

Add simulated location support on testable target configuration #6187

woin2ee commented Apr 14, 2024

fortmarek left a comment

fortmarek Apr 16, 2024

woin2ee Apr 16, 2024

woin2ee Apr 16, 2024 •

edited

fortmarek Apr 19, 2024

woin2ee commented Apr 16, 2024

fortmarek left a comment

fortmarek Apr 19, 2024

fortmarek Apr 19, 2024

woin2ee Apr 20, 2024

fortmarek Apr 19, 2024

woin2ee Apr 20, 2024

fortmarek commented Apr 22, 2024

allcontributors bot commented Apr 22, 2024

fortmarek commented Apr 22, 2024

woin2ee commented Apr 22, 2024

Add simulated location support on testable target configuration #6187

Add simulated location support on testable target configuration #6187

Conversation

woin2ee commented Apr 14, 2024

Short description 📝

How to test the changes locally 🧐

Contributor checklist ✅

Reviewer checklist ✅

fortmarek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woin2ee Apr 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woin2ee commented Apr 16, 2024

fortmarek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fortmarek commented Apr 22, 2024

allcontributors bot commented Apr 22, 2024

fortmarek commented Apr 22, 2024

woin2ee commented Apr 22, 2024

woin2ee Apr 16, 2024 •

edited