-
Notifications
You must be signed in to change notification settings - Fork 4
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
[Snapshot-1544] Update the Tag snapshot #514
Conversation
@@ -6,40 +6,35 @@ | |||
// Copyright © 2023 Adevinta. All rights reserved. | |||
// | |||
|
|||
import UIKit | |||
@testable import SparkCore | |||
|
|||
struct TagSutSnapshotTests { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this struct is strange.
SUT stands for Service Under Tests and is usually given to the variable name of the service being tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replace SUT to configuration
for scenario in scenarios { | ||
let suts = scenario.sut(isSwiftUIComponent: true) | ||
for sut in suts { | ||
let view = TagView(theme: self.theme) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case the view is the SUT and not the variations.
/// - content: icon + text | ||
/// - mode: all | ||
/// - size: default | ||
private func test1(isSwiftUIComponent: Bool) -> [TagSutSnapshotTests] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the function could be more descriptive, something like is in the comment.
Maybe the could be renamed to something like "scenarioAllIntents" .. etc. When I see a function starting with "test", I immediately assume, that it is a test.
import SwiftUI | ||
|
||
enum TagScenarioSnapshotTests: String, CaseIterable { | ||
case test1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the scenarios could be more descriptive, see below.
e804d6b
to
f896762
Compare
Here the new strategy for the Snapshot !
This work was done with Elisa.
The link of the associated Google sheet: here
The new tests correspond to the list of tests define inside the Google sheet.
We have 5 scenarios for the Tag.
At the end, we have now 39 images for each framworks instead of 486...
The PR for the Snapshot testing is here.