-
Notifications
You must be signed in to change notification settings - Fork 13
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
[WIP] Step description + Screenshots #142
base: develop
Are you sure you want to change the base?
[WIP] Step description + Screenshots #142
Conversation
…application state & ElementAttributes
Hello! Thanks for taking the time to do this :) it would be super if you could create issues to discuss changes like this before working on changes because if we decide not to incorporate this then you will have wasted your time |
@@ -11,7 +11,7 @@ import TABTestKit | |||
final class BiometricLogin_AuthenticationFailureTests: TABTestCase, SystemPreferencesContext { | |||
|
|||
override func preLaunchSetup(_ launch: @escaping () -> Void) { | |||
resetAllPrivacyPrompts() | |||
resetAllPrivacyPrompts().execute() |
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'm not really a fan of having to call execute
all over the place :/
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 get that, the problem is that I need to return a description. Suggestions are welcome.
TABTestKit/Classes/TABTestCase.swift
Outdated
@@ -81,6 +81,11 @@ open class TABTestCase: XCTestCase, DefaultContexts { | |||
// You can find this attachment in the .xcresult bundle (usually Derived Data). | |||
let attachment = createFailureAttachment(description: description, filePath: filePath, lineNumber: lineNumber) | |||
add(attachment) | |||
|
|||
if App.shared.screenshotOption.contains(.onFailure) { |
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'm pretty sure XCUI already adds screenshots when a failure happens anyway?
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.
Have to check that, will come back to that
@@ -21,83 +16,40 @@ public typealias And = Step | |||
/// | |||
/// Do not use this Step type directly, instead use one of the typealiases above, like Given, When, Then or And. |
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.
If we do go ahead with these changes, the docs will need updating
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 know, that's in the TODO
in the description of this pr. Together with extend tests. Because I first wanted to pitch this idea to you guys before finishing this task
TABTestKit/Classes/BDD/Step.swift
Outdated
public typealias When = Step | ||
public typealias Then = Step | ||
public typealias And = Step | ||
open class Given: Step {} |
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.
Wondering if instead of classes we could make Step a protocol instead, and then have Given/When/Then/And conform to Step? That way you'd still have lightweight structs but you'd be able to tell the difference between Given/When etc
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.
Great addition, the only thing left was storing the current step, so I decided to move that to TABTestCase, so now you can access it like this: TABTestCase.current?.step
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.
Ahh yes of course. What about just TABTestCase.currentStep? It can be a static property on TABTestCase rather than an instance property on TABTestCase, if that makes sense?
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.
fine with me :) Processed feedback
What's in this PR?
This pr's adds a description for every step & adds the ability to create screenshots
As example, this is what the current unit tests generate:
TODO
Pre-merge checklist
Before merging any PR, please check the following common things that should be done beforehand. These aren't all always required, so just check the box if it doesn't apply.
TABTestKit
target, notPods-TABTestKit_Example
etc.pod install
to ensure that the latest changes are in the Example project. Without this, Carthage might not see the latest changes.CHANGELOG
. For any changes pending a release, add to the Pending section. For releases, move everything pending to the release section.README
. Add info for any new features, update existing info for anything that's changed or needs extra info.