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

Allows passing BuildableIdentifier String to BuildableReference initializer #605

Merged
merged 2 commits into from
May 1, 2021

Conversation

freddi-kit
Copy link
Contributor

@freddi-kit freddi-kit commented Apr 12, 2021

Short description 📝

At BuildableReference, it would be better to pass buildableIdentifier String by the user on some case

For example, when Xcode adding testing for Local Swift Package in scheme like below image,

(MyLibrary is a local swift package)

The (AppName).xcscheme will be like the below code.

<Testables>
   <TestableReference
      skipped = "NO">
      <BuildableReference
         BuildableIdentifier = "primary"
         BlueprintIdentifier = "96072DB32624C6A200AB94A5" # <--- HERE
         BuildableName = "freddiTestAppTests.xctest"
         BlueprintName = "freddiTestAppTests"
         ReferencedContainer = "container:freddiTestApp.xcodeproj">
      </BuildableReference>
   </TestableReference>
   <TestableReference
      skipped = "NO">
      <BuildableReference
         BuildableIdentifier = "primary"
         BlueprintIdentifier = "MyLibraryTests" # <--- HERE
         BuildableName = "MyLibraryTests"
         BlueprintName = "MyLibraryTests"
         ReferencedContainer = "container:MyLibrary">
      </BuildableReference>
   </TestableReference>
</Testables>
FYI: `MyLibrary`'s Package.swift
// swift-tools-version:5.3
// The swift-tools-version declares the minimum version of Swift required to build this package.

import PackageDescription

let package = Package(
    name: "MyLibrary",
    products: [
        // Products define the executables and libraries a package produces, and make them visible to other packages.
        .library(
            name: "MyLibrary",
            targets: ["MyLibrary"]),
    ],
    dependencies: [
        // Dependencies declare other packages that this package depends on.
        // .package(url: /* package url */, from: "1.0.0"),
    ],
    targets: [
        // Targets are the basic building blocks of a package. A target can define a module or a test suite.
        // Targets can depend on other targets in this package, and on products in packages this package depends on.
        .target(
            name: "MyLibrary",
            dependencies: []),
        .testTarget(
            name: "MyLibraryTests",
            dependencies: ["MyLibrary"]),
    ]
)

As you can see, the application test's BlueprintIdentifier is for reference of xctest. But MyLibrary's BlueprintIdentifier is not the same format with main app one(96072DB32624C6A200AB94A5). It is the name of test target of MyLibrary.

Currently, BlueprintIdentifier's initalizer only set BlueprintIdentifier via received PBXObject like

        public init(...,
                    blueprint: PBXObject,
                    ...) {
            ...
            self.blueprint = .reference(blueprint.reference)
            ...
        }

However, it is hard to pass the actual PBXObject's reference when adding BuildableReference for testing Local Swift Package.

Solution 📦

So I added an initializer that receives BlueprintIdentifier as a string to make it easy to set as expected.

        public init(...,
                    blueprintIdentifier: String,
                    ...) {
            ...
            self.blueprint = .string(blueprintIdentifier)
            ...
        }

Implementation 👩‍💻👨‍💻

  • add new initalizer for BuildableReference which receives BuildableIdentifier string

@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #605 (f793723) into main (6943d5c) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #605      +/-   ##
==========================================
- Coverage   84.24%   84.18%   -0.06%     
==========================================
  Files         154      154              
  Lines        8682     8688       +6     
==========================================
  Hits         7314     7314              
- Misses       1368     1374       +6     
Impacted Files Coverage Δ
...XcodeProj/Scheme/XCScheme+BuildableReference.swift 76.92% <0.00%> (-10.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6943d5c...f793723. Read the comment docs.

Copy link
Contributor

@giginet giginet left a comment

Choose a reason for hiding this comment

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

I think this change makes sense 👍

@freddi-kit
Copy link
Contributor Author

freddi-kit commented Apr 23, 2021

Can other also review it? 👀

Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks for adding this @freddi-kit

Looking at the code, it seemed .string() references were exclusives used when deserialising the scheme and making a new one relied on PBXObjects hence why it wasn't exposed - thanks for the additional context pointing out the use-case where Swift Package integrations work differently requiring manually setting the string directly.

@freddi-kit
Copy link
Contributor Author

Can someone help to merge it?

@kwridan
Copy link
Collaborator

kwridan commented Apr 30, 2021

Sorry for the hassle @freddi-kit - mind rebasing on latest main + updating the change log?

@freddi-kit freddi-kit force-pushed the add/initalizer-buildable-identifier branch from f793723 to 32774ad Compare May 1, 2021 11:09
@codecov-commenter
Copy link

codecov-commenter commented May 1, 2021

Codecov Report

Merging #605 (32774ad) into main (77b7102) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #605      +/-   ##
==========================================
- Coverage   84.20%   84.14%   -0.06%     
==========================================
  Files         154      154              
  Lines        8700     8706       +6     
==========================================
  Hits         7326     7326              
- Misses       1374     1380       +6     
Impacted Files Coverage Δ
...XcodeProj/Scheme/XCScheme+BuildableReference.swift 76.92% <0.00%> (-10.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77b7102...32774ad. Read the comment docs.

@freddi-kit
Copy link
Contributor Author

I changed CHANGELOG and rebased main, Thank you!

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.

4 participants