Support test target for local Swift Package #1074

freddi-kit · 2021-05-05T17:54:04Z

note: This PR needs to wait for releasing new version of tuist/XcodeProj with tuist/XcodeProj#605

CHANGELOG.md

giginet · 2021-05-06T14:12:51Z

Package.resolved

-          "branch": null,
-          "revision": "94e55232d227f9d78b811c98cb2e5d0cbd08987b",
-          "version": "7.22.0"
+          "branch": "main",


We shouldn't use an unstable version.

This PR may depend on this patch.
Allows passing BuildableIdentifier String to BuildableReference initializer by freddi-kit · Pull Request #605 · tuist/XcodeProj

It's better to distribute newer XcodeProj.

I also wanna use new stable XcoreProj version with my patch, so I will modify it after new version released.
That's why this Pull Request is still Draft.

Sources/XcodeGenKit/SchemeGenerator.swift

kateinoigakukun

I have two comments about this PR:

Is this feature limited to test targets? I think we can relax the limitation for non-test targets.
Since this change, ProjectName/TargetName syntax represents a reference to a .xcodeproj or a SwiftPM package, right? Then, what happens when the given project.yml is like below:

projectReferences:
  Foo:
    path: ./Foo.xcodeproj
packages:
  Foo:
    url: ./Elsewhere/Foo

In your implementation, SwiftPM package version will be chosen, but it's not clear for users.
So we should have a single namespace for external project names or we can have a new syntax to represent a target reference for SwiftPM, but I have no idea to represent such structure with current project.yml syntax, sorry

freddi-kit · 2021-05-07T14:05:53Z

@kateinoigakukun

Thanks, basically, I totally agree with your ideas.

About 1., we can expand to another thing which using TargetReference type like coverageTargets, BuildTarget and more. It would be useful and not need to make more changes if it would be supported by many places.

And currently, my changes cause ambiguous format ("/" for project and package), as you explained 2, so I'm concerning change the format a little more clear.

freddi-kit · 2021-05-07T14:08:27Z

I have an idea for solving these 2 concerns: Having a new Yaml(JSON) format for TargetReference Type.

I feel this discussion is based on how TargetReference type is treated. Currently, TargetReference is generated by one string like the below code from yml file.

XcodeGen/Sources/ProjectSpec/TargetReference.swift

Lines 20 to 36 in 3757f82

    
           public init(_ string: String) throws { 
        
               let paths = string.split(separator: "/") 
        
               switch paths.count { 
        
               case 2: 
        
                   location = .project(String(paths[0])) 
        
                   name = String(paths[1]) 
        
               case 1: 
        
                   location = .local 
        
                   name = String(paths[0]) 
        
               default: 
        
                   throw SpecParsingError.invalidTargetReference(string) 
        
               } 
        
           } 
        
           public static func local(_ name: String) -> TargetReference { 
        
               TargetReference(name: name, location: .local) 
        
           }

And used for many places like testTartgets and coverageTargets.

I think we can change TargetReference to have new format like below and make it convertible from Yaml(JSON)

- project: Project/ProjectTest
- package: Package/PackageTest
- local: LocalTest

What do you think about it?

kateinoigakukun · 2021-05-07T14:23:17Z

+1 for adding a new syntax in TargetReference 👍

freddi-kit · 2021-05-25T16:53:23Z

@kateinoigakukun @giginet
I changed code from @kateinoigakukun 's comment:

use new TargetReference fomat for specifying local swift package test case on Test scheme (targets and coverageTargets)

Please help review it, Thanks

Docs/ProjectSpec.md

mthole · 2021-08-02T17:38:46Z

@yonaskolb @freddi-kit Thanks for your work here! Can we get this work landed soon? I am running into this exact problem and a fix in mainline XcodeGen would be much appreciated!

freddi-kit · 2021-09-23T20:54:10Z

Ops, sorry for leaving it so long time. I worked on it but I stopped because it has some difficulty to achieve it. This change make the project spec confusable so I was finding a way to solve it. I have the motivation and I probably find out the way to do it without making spec complex. Let me touch you until this week!

razor313 · 2021-10-19T14:19:09Z

@freddi-kit Could you find a way in order to solve it in an easy way? We're waiting for it. :)))

Docs/ProjectSpec.md

freddi-kit · 2021-10-20T13:24:22Z

@freddi-kit Could you find a way in order to solve it in an easy way? We're waiting for it. :)))

I'm sorry for waiting you for long time, this difficulty is about the internal API things and I just commited!

dehlen · 2021-10-26T15:59:47Z

Really looking forward to this. Any idea when you'll find time going over these changes @yonaskolb ?

yonaskolb

Thanks for this again @freddi-kit, looks good to me! Only thing I'm slightly hesitant about is the syntax for declaring a test target is a bit verbose with the nested target.

testTargets:
         - target:
             package: XcodeGen/XcodeGenKitTests
           parallelizable: true

One other option would be to decode the TestableTargetReference at the top level, but I think how you have it is ok, and makes for cleaner code.

testTargets:
         - package: XcodeGen/XcodeGenKitTests
           parallelizable: true

What I would like to see though is some parsing and validation tests to cover this (SpecLoadingTests.testProjectSpecParser, ProjectSpectTests.testValidation), and then we can get this merged in! 👍

yonaskolb · 2021-11-03T08:32:23Z

Docs/ProjectSpec.md

      targets: 
        - Tester1 
        - name: Tester2
          parallelizable: true
          randomExecutionOrder: true
          skippedTests: [Test/testExample()]
+        - target: 
+          package: APIClient/APIClientTests


Suggested change

package: APIClient/APIClientTests

package: APIClient/APIClientTests

This is not really consistent in the way the current yml is constructed. Is there a specific reason why this is indented here?

tww0003 · 2021-11-04T17:01:49Z

Docs/ProjectSpec.md

+A target can be one of a 2 types:
+
+- **name**: **String** - The name of the target. If you want to specify local swift package, please use `target:`.
+- **target**: **[Testable Target Reference](#target-reference)** - The information of the target. You can specify more detailed information than `name:`.


This link to the Testable Target section is incorrect here. It should be:
**[Testable Target Reference](#testable-target-reference)**

that's right, nice find 👍

jakeheis · 2021-11-12T20:26:53Z

@freddi-kit I'd be happy to open up a PR with the requested parsing and validation tests if you'd like some help getting this across the finish line

mthole · 2021-11-22T22:54:08Z

FYI, I updated our project to point to this branch and successfully used the new functionality! 🚀 Looking forward to getting this in the mainline

razor313 · 2021-12-07T15:10:33Z

@freddi-kit I see the PR has been approved, Could you please update/merge it.

skofgar · 2022-01-14T21:17:56Z

Hi everyone, I was curious if this is still planned to be updated/merged at some point?

skofgar · 2022-01-14T21:19:25Z

Sources/ProjectSpec/TestTargeReference.swift

+import Foundation
+import JSONUtilities
+
+public struct TestableTargetReference: Hashable {


Instead of introducing TestableTargetReference would it be possible to just enhance TargetReference. I'm asking because build targets can also be in local Swift Packages and not just test targets?

it is because we already have packages: things which is almost same with targets: but for only swift package. And I tried to do so but there is a problem that it make confuses with packages. I wanna keep them clear as possible

ghost · 2022-01-20T09:39:22Z

I used truecaller/XcodeGen.git, but I don't see any package in testTargets show on .xcscheme

    scheme:
      testTargets:
        - target:
          package: HappyTests
        - target:
          local: DebugToolTests
        - target:
          local: Modules/HappyTests
        - target:
          package: Modules/ZTests

but this way work.

    scheme:
      testTargets:
        - target:
            package: Modules/HappyTests

fousa · 2022-02-02T07:20:39Z

Is there any progress on this ticket? I would be happy to help.

We currently have a modular setup like this:

App
AppTests
Modules/Network
Modules/Network/NetworkTests
Modules/Flow
Modules/Flow/FlowTests

And I would like to create an AllTests scheme that runs the following tests:

AppTests
Modules/Network/NetworkTests
Modules/Flow/FlowTests

I think I have the perfect example to try out this pull request, so let me know if I can help!

freddi-kit · 2022-02-02T13:38:13Z

I'm sorry for making you waiting so long time, I finally fixed comments from @yonaskolb, please check.

freddi-kit · 2022-02-02T13:42:24Z

ops. I closed PR by mistake and it looks i cannot reopen it, please wait

freddi-kit · 2022-02-02T14:07:12Z

Reopened #1169

freddi-kit commented May 5, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

freddi-kit force-pushed the test-local-spm branch from 28b1cc2 to 5474cc5 Compare May 6, 2021 13:44

giginet reviewed May 6, 2021

View reviewed changes

kateinoigakukun requested changes May 7, 2021

View reviewed changes

freddi-kit marked this pull request as ready for review May 25, 2021 16:50

freddi-kit requested review from giginet and kateinoigakukun May 25, 2021 16:51

yonaskolb reviewed Jun 18, 2021

View reviewed changes

Docs/ProjectSpec.md Outdated Show resolved Hide resolved

freddi-kit commented Oct 20, 2021

View reviewed changes

Docs/ProjectSpec.md Outdated Show resolved Hide resolved

freddi-kit commented Oct 20, 2021

View reviewed changes

Docs/ProjectSpec.md Outdated Show resolved Hide resolved

freddi-kit commented Oct 20, 2021

View reviewed changes

Docs/ProjectSpec.md Outdated Show resolved Hide resolved

Docs/ProjectSpec.md Outdated Show resolved Hide resolved

freddi-kit requested a review from yonaskolb October 20, 2021 13:24

yonaskolb reviewed Nov 3, 2021

View reviewed changes

tww0003 reviewed Nov 4, 2021

View reviewed changes

nevillco mentioned this pull request Dec 27, 2021

Reference Local SPM Targets from XcodeGen #1160

Open

skofgar reviewed Jan 14, 2022

View reviewed changes

vasily-popov mentioned this pull request Jan 17, 2022

Support test target for local Swift Package truecaller/XcodeGen#1

Merged

skofgar mentioned this pull request Jan 25, 2022

Remove TestableTargetReference and enhance TargetReference instead (related to #1074) #1168

Closed

freddi-kit closed this Feb 2, 2022

freddi-kit force-pushed the test-local-spm branch from 66ff567 to c62277f Compare February 2, 2022 13:41

freddi-kit mentioned this pull request Feb 2, 2022

Support test target for local Swift Package #1169

Merged

skofgar mentioned this pull request Feb 4, 2022

Remove TestableTargetReference and enhance TargetReference instead (related to #1169) #1170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support test target for local Swift Package #1074

Support test target for local Swift Package #1074

freddi-kit commented May 5, 2021

giginet May 6, 2021

freddi-kit May 6, 2021 •

edited

Loading

kateinoigakukun left a comment •

edited

Loading

freddi-kit commented May 7, 2021

freddi-kit commented May 7, 2021 •

edited

Loading

kateinoigakukun commented May 7, 2021

freddi-kit commented May 25, 2021

mthole commented Aug 2, 2021

freddi-kit commented Sep 23, 2021

razor313 commented Oct 19, 2021

freddi-kit commented Oct 20, 2021

dehlen commented Oct 26, 2021

yonaskolb left a comment •

edited

Loading

yonaskolb Nov 3, 2021

fousa Feb 2, 2022

tww0003 Nov 4, 2021

freddi-kit Feb 2, 2022

jakeheis commented Nov 12, 2021

mthole commented Nov 22, 2021

razor313 commented Dec 7, 2021 •

edited

Loading

skofgar commented Jan 14, 2022 •

edited

Loading

skofgar Jan 14, 2022 •

edited

Loading

freddi-kit Feb 2, 2022

ghost commented Jan 20, 2022 •

edited by ghost

Loading

fousa commented Feb 2, 2022 •

edited

Loading

freddi-kit commented Feb 2, 2022

freddi-kit commented Feb 2, 2022

freddi-kit commented Feb 2, 2022

	package: APIClient/APIClientTests
	package: APIClient/APIClientTests

Support test target for local Swift Package #1074

Support test target for local Swift Package #1074

Conversation

freddi-kit commented May 5, 2021

giginet May 6, 2021

Choose a reason for hiding this comment

freddi-kit May 6, 2021 • edited Loading

Choose a reason for hiding this comment

kateinoigakukun left a comment • edited Loading

Choose a reason for hiding this comment

freddi-kit commented May 7, 2021

freddi-kit commented May 7, 2021 • edited Loading

kateinoigakukun commented May 7, 2021

freddi-kit commented May 25, 2021

mthole commented Aug 2, 2021

freddi-kit commented Sep 23, 2021

razor313 commented Oct 19, 2021

freddi-kit commented Oct 20, 2021

dehlen commented Oct 26, 2021

yonaskolb left a comment • edited Loading

Choose a reason for hiding this comment

yonaskolb Nov 3, 2021

Choose a reason for hiding this comment

fousa Feb 2, 2022

Choose a reason for hiding this comment

tww0003 Nov 4, 2021

Choose a reason for hiding this comment

freddi-kit Feb 2, 2022

Choose a reason for hiding this comment

jakeheis commented Nov 12, 2021

mthole commented Nov 22, 2021

razor313 commented Dec 7, 2021 • edited Loading

skofgar commented Jan 14, 2022 • edited Loading

skofgar Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

freddi-kit Feb 2, 2022

Choose a reason for hiding this comment

ghost commented Jan 20, 2022 • edited by ghost Loading

fousa commented Feb 2, 2022 • edited Loading

freddi-kit commented Feb 2, 2022

freddi-kit commented Feb 2, 2022

freddi-kit commented Feb 2, 2022

freddi-kit May 6, 2021 •

edited

Loading

kateinoigakukun left a comment •

edited

Loading

freddi-kit commented May 7, 2021 •

edited

Loading

yonaskolb left a comment •

edited

Loading

razor313 commented Dec 7, 2021 •

edited

Loading

skofgar commented Jan 14, 2022 •

edited

Loading

skofgar Jan 14, 2022 •

edited

Loading

ghost commented Jan 20, 2022 •

edited by ghost

Loading

fousa commented Feb 2, 2022 •

edited

Loading