Better Support Loading Legacy Schemes #194
Conversation
In preparation for adding additional scheme integration tests, remove the subject property and change test_init_initializesTheSchemeCorrectly() to load/assert the scheme directly (the property was otherwise unused). This gives us one independent function for verifying reading and one independent function for verifying writing. Originally I called the assert function in the initModel closure in the test_write() method but decided against that because I didn't like the read test being a side effect of the write test.
|
Thanks @briantkelley for improving the XCScheme initialization to match the Xcode behavior. We did that for the projects but we forgot about the schemes. Regarding your comment on the breaking change I've updated your PR to have |
Fixtures/Schemes/AIS.xcscheme
Outdated
| @@ -0,0 +1,43 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <Scheme | |||
There was a problem hiding this comment.
Out of curiosity, what does AIS stand for?
There was a problem hiding this comment.
Automation Infrastructure Shared, which is an internal library for testing support. I wasn't sure if the name of a file loaded during a test case had much value, but I'd be happy to rename it to something a bit more useful. Perhaps LotsOfMissingInformation.xcscheme?
| @@ -34,6 +34,14 @@ final class XcodeGenTests: XCTestCase { | |||
| try packageSwiftWithLocalDependency.write(to: packageSwiftPath.url, | |||
| atomically: true, | |||
| encoding: .utf8) | |||
| // Testing xcodegen as part of xcproj creates a circular dependency; fix up source files affected by API changes | |||
| let projectGeneratorTestsPath = clonePath + Path("Tests/XcodeGenKitTests/ProjectGeneratorTests.swift") | |||
There was a problem hiding this comment.
I'm sorry that how we test xcproj is not well documented @briantkelley. XcodeGenTests is part of a suite of tests, integration tests, that we don't run from PRs. These tests are run after very merge into master to know if there was any change that is breaking. We test that by using XcodeGen and updating the Package.swift to point to our changes.
If you revert these changes this PR should pass (ignore the local failures since they are expected)
There was a problem hiding this comment.
Thanks for the info. I saw swift test failed and thought I should fix it. But merging it in to the 2.0 branch and updating the framework makes so much more sense.
Xcode accepts just about any scheme file, filling the model with default values if necessary. Update the XCScheme XML initialization to better match Xcode's behavior. MinimalInformation.xcscheme was taken from a large codebase with 279 Xcode projects. After simply opening the scheme in the Scheme Editor and closing the sheet, Xcode rewrites the file with default values. These default values were integrated in to the XCScheme loading code as fallbacks for when the attribute or element doesn't exist. Also note that the "buildableProductRunnable" property/element isn't required. It's possible to select "None" for the executable in the Scheme Editor, in which case Xcode won't generate the element. Also update the XCScheme integration test to test reading and writing the underspecified scheme to ensure it can be successfully loaded with the correct default values.
|
Thanks for the feedback @pepibumur. I renamed the scheme to something a bit more descriptive and removed the hack for the tests given the branching strategy. |
Short description 📝
Xcode accepts just about any scheme file, filling the model with default values if necessary. xcproj rejected a number of schemes Xcode accepted in a codebase with 279 Xcode projects.
Solution 📦
Update the XCScheme XML initialization to better match Xcode's behavior.
Note that the
buildableProductRunnableproperty/element isn't required. It's possible to select "None" for the executable in the Scheme Editor, in which case Xcode won't generate the element.The xcproj tests test xcodegen, which creates a circular dependency for validation. Because this change breaks the API, we have to fix up the xcodegen source files until it adopts the version of xcproj that contains this change.
Implementation 👩💻👨💻
This change is