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

Add Support for Test Plans #1936

Merged
merged 15 commits into from Nov 16, 2020
Merged

Add Support for Test Plans #1936

merged 15 commits into from Nov 16, 2020

Conversation

iteracticman
Copy link
Collaborator

@iteracticman iteracticman commented Oct 21, 2020

Resolves [NO_ISSUE]

Short description πŸ“

Tuist has no support for Test Plans yet, but we need it to be able to adopt it for our project, so I implemented it.

Solution πŸ“¦

I tried to not introduce any breaking changes and keep the solution as simple as possible.

Implementation πŸ‘©β€πŸ’»πŸ‘¨β€πŸ’»

  • Add ProjectDescription.TestPlanList and add it to ProjectDescription.TestAction
  • Add ProjectDescription.TestAction.testPlans factory method to create Test Actions that have Test Plans instead of Test Targets and restructured initializers along the way
  • Add TuistCore.TestPlan and extend TuistCore.TestAction as well as TuistGenerator.SchemesGenerator to include Test Plans, if present in Project Description
  • Add a very basic test to SchemesGeneratorTests
  • Adapt ios_app_with_custom_scheme fixture and test case
  • Update documentation
  • Update CHANGELOG

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 submitting this @iteracticman and great first contribution to the project!

I think you covered most of the technical bits needed to get this feature working and should be good to merge with some minor tweaks.

Some additional bits to complete the PR:

Thanks again @iteracticman - feel free to post back if you need a hand with any of those.

@pepicrft pepicrft changed the base branch from master to main October 28, 2020 07:46
@iteracticman iteracticman marked this pull request as ready for review October 30, 2020 10:26
@iteracticman
Copy link
Collaborator Author

iteracticman commented Oct 30, 2020

Thanks @kwridan, everything should be ready now. Please check.

Comment on lines 12 to 17
let customAppSchemeWithTestPlans = Scheme(name: "Workspace-App-With-TestPlans",
shared: true,
buildAction: BuildAction(targets: [.project(path: "App", target: "App")], preActions: []),
testAction: .testPlans(default: "DefaultTestPlan.xctestplan", other: ["OtherTestPlan.xctestplan", "YetAnotherTestPlan.xctestplan", "NonExistentTestPlan.xctestplan"]),
runAction: RunAction(executable: .project(path: "App", target: "App")),
archiveAction: ArchiveAction(configurationName: "Debug", customArchiveName: "Something2"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

new scheme

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 the updates @iteracticman

Copy link
Contributor

@pepicrft pepicrft 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 bringing this feature to the project @iteracticman. I left some final comments. I think we can merge once they are addressed πŸš€

CHANGELOG.md Outdated Show resolved Hide resolved
Sources/ProjectDescription/TestAction.swift Outdated Show resolved Hide resolved
@@ -101,4 +130,42 @@ public struct TestAction: Equatable, Codable {
language: language,
region: region)
}

public static func testPlans(default: Path,
Copy link
Contributor

Choose a reason for hiding this comment

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

Dumping an idea here... I like that we have now .testPlans... for initializing the TestActions. What about sticking to the same idea when initializing test actions for targets?

let first: TestAction = .testPlans(default: "...")
let second: TestAction = .targets(targets: [])

And I'd add then a deprecation annotation to the public init to nudge people to use the new constructor. When you do that make sure you test that the warning doesn't break the loading of the manifest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like that. will do.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried, but it would require me to add two more static functions because we need to support both config: PresetBuildConfiguration and configurationName: String.

It's already quite redundant, so maybe it's better to refactor configuration handling a bit first to get rid of that redundancy?

Sources/ProjectDescription/TestPlan.swift Outdated Show resolved Hide resolved
@pepicrft pepicrft merged commit b1c0c9e into tuist:main Nov 16, 2020
@iteracticman iteracticman deleted the test-plans branch November 17, 2020 05:16
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.

None yet

4 participants