Skip to content

Support test screenshot configuration options #82

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support test screenshot configuration options #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 28 additions & 8 deletions Sources/XcodeGraph/Models/ProjectOptions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ extension Project.Options {
testLanguage: String? = nil,
testRegion: String? = nil,
testScreenCaptureFormat: ScreenCaptureFormat? = nil,
captureScreenshotsAutomatically: Bool,
deleteScreenshotsWhenEachTestSucceeds: Bool,
Comment on lines +62 to +63
Copy link
Member

Choose a reason for hiding this comment

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

This technically constitutes a breaking change which we are trying to avoid.

I'd consider adding the new options to TestingOptions instead.

Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest to model these options more closer to how they are modeled in XcodeProj: https://github.com/tuist/XcodeProj/blob/9f26d78d72ef40dd2e35f624bbcde1e3b28762cf/Sources/XcodeProj/Scheme/XCScheme%2BTestAction.swift#L42

Copy link
Author

@sanghun0724 sanghun0724 Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'd consider adding the new options to TestingOptions instead.

got it. Will make the change right away.

Copy link
Author

@sanghun0724 sanghun0724 Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'd suggest to model these options more closer to how they are modeled in XcodeProj:
https://github.com/tuist/XcodeProj/blob/9f26d78d72ef40dd2e35f624bbcde1e3b28762cf/Sources/XcodeProj/Scheme/XCScheme%2BTestAction.swift#L42

What do you think about this comment? I believe that structuring variables closer to the xcodeproj model as you suggested might make it less user-friendly from an user perspective. (There are no such Boolean values for captureScreenshotsAutomatically and deleteScreenshotsWhenEachTestSucceeds)

Copy link
Member

Choose a reason for hiding this comment

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

captureScreenshotsAutomatically definitely should be an enum as you can select between screenshots and recordings. That wasn't the case when that comment was posted.

Also, XcodeGraph is not something Tuist users directly interact with and I think we benefit from keeping the way we structure things somewhat similar. So, I'd still be for aligning with XcodeProj.

runLanguage: String? = nil,
runRegion: String? = nil
)
Expand Down Expand Up @@ -100,7 +102,7 @@ extension Project.Options {
extension Project.Options {
public var targetSchemesGrouping: AutomaticSchemesOptions.TargetSchemesGrouping? {
switch automaticSchemesOptions {
case let .enabled(targetSchemesGrouping, _, _, _, _, _, _, _):
case let .enabled(targetSchemesGrouping, _, _, _, _, _, _, _, _, _):
return targetSchemesGrouping
case .disabled:
return nil
Expand All @@ -109,7 +111,7 @@ extension Project.Options {

public var codeCoverageEnabled: Bool {
switch automaticSchemesOptions {
case let .enabled(_, codeCoverageEnabled, _, _, _, _, _, _):
case let .enabled(_, codeCoverageEnabled, _, _, _, _, _, _, _, _):
return codeCoverageEnabled
case .disabled:
return false
Expand All @@ -118,7 +120,7 @@ extension Project.Options {

public var testingOptions: TestingOptions {
switch automaticSchemesOptions {
case let .enabled(_, _, testingOptions, _, _, _, _, _):
case let .enabled(_, _, testingOptions, _, _, _, _, _, _, _):
return testingOptions
case .disabled:
return []
Expand All @@ -127,7 +129,7 @@ extension Project.Options {

public var testLanguage: String? {
switch automaticSchemesOptions {
case let .enabled(_, _, _, language, _, _, _, _):
case let .enabled(_, _, _, language, _, _, _, _, _, _):
return language
case .disabled:
return nil
Expand All @@ -136,7 +138,7 @@ extension Project.Options {

public var testRegion: String? {
switch automaticSchemesOptions {
case let .enabled(_, _, _, _, region, _, _, _):
case let .enabled(_, _, _, _, region, _, _, _, _, _):
return region
case .disabled:
return nil
Expand All @@ -145,16 +147,34 @@ extension Project.Options {

public var testScreenCaptureFormat: ScreenCaptureFormat? {
switch automaticSchemesOptions {
case let .enabled(_, _, _, _, _, testScreenCaptureFormat, _, _):
case let .enabled(_, _, _, _, _, testScreenCaptureFormat, _, _, _, _):
return testScreenCaptureFormat
case .disabled:
return nil
}
}

public var captureScreenshotsAutomatically: Bool {
switch automaticSchemesOptions {
case let .enabled(_, _, _, _, _, _, captureScreenshotsAutomatically, _, _, _):
return captureScreenshotsAutomatically
case .disabled:
return false
}
}

public var deleteScreenshotsWhenEachTestSucceeds: Bool {
switch automaticSchemesOptions {
case let .enabled(_, _, _, _, _, _, _, deleteScreenshotsWhenEachTestSucceeds, _, _):
return deleteScreenshotsWhenEachTestSucceeds
case .disabled:
return false
}
}

public var runLanguage: String? {
switch automaticSchemesOptions {
case let .enabled(_, _, _, _, _, _, language, _):
case let .enabled(_, _, _, _, _, _, _, _, language, _):
return language
case .disabled:
return nil
Expand All @@ -163,7 +183,7 @@ extension Project.Options {

public var runRegion: String? {
switch automaticSchemesOptions {
case let .enabled(_, _, _, _, _, _, _, region):
case let .enabled(_, _, _, _, _, _, _, _, _, region):
return region
case .disabled:
return nil
Expand Down
10 changes: 10 additions & 0 deletions Sources/XcodeGraph/Models/TestAction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public struct TestAction: Equatable, Codable, Sendable {
public var diagnosticsOptions: SchemeDiagnosticsOptions
public var language: String?
public var region: String?
public var captureScreenshotsAutomatically: Bool
public var deleteScreenshotsWhenEachTestSucceeds: Bool
public var preferredScreenCaptureFormat: ScreenCaptureFormat?
public var skippedTests: [String]?

Expand All @@ -36,6 +38,8 @@ public struct TestAction: Equatable, Codable, Sendable {
language: String? = nil,
region: String? = nil,
preferredScreenCaptureFormat: ScreenCaptureFormat? = nil,
captureScreenshotsAutomatically: Bool,
deleteScreenshotsWhenEachTestSucceeds: Bool,
testPlans: [TestPlan]? = nil,
skippedTests: [String]? = nil
) {
Expand All @@ -53,6 +57,8 @@ public struct TestAction: Equatable, Codable, Sendable {
self.language = language
self.region = region
self.preferredScreenCaptureFormat = preferredScreenCaptureFormat
self.captureScreenshotsAutomatically = captureScreenshotsAutomatically
self.deleteScreenshotsWhenEachTestSucceeds = deleteScreenshotsWhenEachTestSucceeds
self.skippedTests = skippedTests
}
}
Expand All @@ -77,6 +83,8 @@ public struct TestAction: Equatable, Codable, Sendable {
language: String? = nil,
region: String? = nil,
preferredScreenCaptureFormat: ScreenCaptureFormat? = nil,
captureScreenshotsAutomatically: Bool = false,
deleteScreenshotsWhenEachTestSucceeds: Bool = false,
testPlans: [TestPlan]? = nil,
skippedTests: [String]? = nil
) -> TestAction {
Expand All @@ -94,6 +102,8 @@ public struct TestAction: Equatable, Codable, Sendable {
language: language,
region: region,
preferredScreenCaptureFormat: preferredScreenCaptureFormat,
captureScreenshotsAutomatically: captureScreenshotsAutomatically,
deleteScreenshotsWhenEachTestSucceeds: deleteScreenshotsWhenEachTestSucceeds,
testPlans: testPlans,
skippedTests: skippedTests
)
Expand Down
8 changes: 5 additions & 3 deletions Sources/XcodeGraph/Models/Workspace.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ public struct Workspace: Equatable, Codable {
testingOptions: [],
testLanguage: nil,
testRegion: nil,
testScreenCaptureFormat: nil
testScreenCaptureFormat: nil,
captureScreenshotsAutomatically: true,
deleteScreenshotsWhenEachTestSucceeds: false
),
lastXcodeUpgradeCheck: nil,
renderMarkdownReadme: false
Expand Down Expand Up @@ -131,7 +133,7 @@ extension Workspace {
extension Workspace {
public var codeCoverageMode: GenerationOptions.AutogeneratedWorkspaceSchemes.CodeCoverageMode {
switch generationOptions.autogeneratedWorkspaceSchemes {
case let .enabled(codeCoverageMode, _, _, _, _):
case let .enabled(codeCoverageMode, _, _, _, _, _, _):
return codeCoverageMode
case .disabled:
return .disabled
Expand All @@ -140,7 +142,7 @@ extension Workspace {

public var testingOptions: TestingOptions {
switch generationOptions.autogeneratedWorkspaceSchemes {
case let .enabled(_, testingOptions, _, _, _):
case let .enabled(_, testingOptions, _, _, _, _, _):
return testingOptions
case .disabled:
return []
Expand Down
32 changes: 26 additions & 6 deletions Sources/XcodeGraph/Models/WorkspaceGenerationOptions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ extension Workspace {
testingOptions: TestingOptions = [],
testLanguage: String? = nil,
testRegion: String? = nil,
testScreenCaptureFormat: ScreenCaptureFormat? = nil
testScreenCaptureFormat: ScreenCaptureFormat? = nil,
captureScreenshotsAutomatically: Bool = true,
deleteScreenshotsWhenEachTestSucceeds: Bool = false
)

public var codeCoverageMode: CodeCoverageMode {
switch self {
case let .enabled(codeCoverageMode, _, _, _, _):
case let .enabled(codeCoverageMode, _, _, _, _, _, _):
return codeCoverageMode
case .disabled:
return .disabled
Expand All @@ -39,7 +41,7 @@ extension Workspace {

public var testingOptions: TestingOptions {
switch self {
case let .enabled(_, testingOptions, _, _, _):
case let .enabled(_, testingOptions, _, _, _, _, _):
return testingOptions
case .disabled:
return []
Expand All @@ -48,7 +50,7 @@ extension Workspace {

public var testLanguage: String? {
switch self {
case let .enabled(_, _, language, _, _):
case let .enabled(_, _, language, _, _, _, _):
return language
case .disabled:
return nil
Expand All @@ -57,7 +59,7 @@ extension Workspace {

public var testRegion: String? {
switch self {
case let .enabled(_, _, _, region, _):
case let .enabled(_, _, _, region, _, _, _):
return region
case .disabled:
return nil
Expand All @@ -66,12 +68,30 @@ extension Workspace {

public var testScreenCaptureFormat: ScreenCaptureFormat? {
switch self {
case let .enabled(_, _, _, _, testScreenCaptureFormat):
case let .enabled(_, _, _, _, testScreenCaptureFormat, _, _):
return testScreenCaptureFormat
case .disabled:
return nil
}
}

public var captureScreenshotsAutomatically: Bool {
switch self {
case let .enabled(_, _, _, _, _, captureScreenshotsAutomatically, _):
return captureScreenshotsAutomatically
case .disabled:
return false
}
}

public var deleteScreenshotsWhenEachTestSucceeds: Bool {
switch self {
case let .enabled(_, _, _, _, _, _, deleteScreenshotsWhenEachTestSucceeds):
return deleteScreenshotsWhenEachTestSucceeds
case .disabled:
return false
}
}
}

/// Tuist generates a WorkspaceSettings.xcsettings file, setting the related key to the associated value.
Expand Down