-
Notifications
You must be signed in to change notification settings - Fork 1
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
simplified public tests #2
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
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.
Thank you very much for doing this. You are correct of course, and I would have saved myself some issues if I'd done this much earlier!
I've left a few comments as requests for changes just to make things match up with the rest of the project and then I'd be happy to put it in.
import XCTest | ||
import SwiftFSM | ||
|
||
final class SimplifiedPublicTests: XCTestCase { |
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.
Change file and test class name to PublicInterfaceTests, to accompany some changes to other file names that I made in preparation for this (PublicTests -> PublicFSMTests, FSMTests -> InternalFSMTests).
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.
(make sure to pull the latest changes from master first)
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.
should be good now thanks for the feedback
func unlock() { | ||
unlockCalled = true | ||
} | ||
func alarm() { alarmCalled = true } | ||
func thankyou() { thankyouCalled = true } | ||
func lock() { lockCalled = true } |
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.
Inline the unlock() body
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.
ok done
func testPublicInterface() throws { | ||
let sut = try MyClass() | ||
try sut.fsm.handleEvent(.coin) | ||
XCTAssertTrue(sut.unlockCalled) | ||
XCTAssertFalse(sut.thankyouCalled) | ||
try sut.fsm.handleEvent(.coin) | ||
XCTAssertTrue(sut.thankyouCalled) | ||
try sut.fsm.handleEvent(.coin) | ||
XCTAssertFalse(sut.lockCalled) | ||
try sut.fsm.handleEvent(.pass) | ||
XCTAssertTrue(sut.lockCalled) | ||
try sut.fsm.handleEvent(.pass) | ||
XCTAssertTrue(sut.alarmCalled) |
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.
Add a blank line after each handleEvent()/XCTAssert... pair
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.
Update indents to four spaces to match rest of project
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.
oh yes my settings are different, keep forgetting that. Changed it!
I think it is important to test the public interface without the need for @testable
I think it is important to test the public interface without the need for @testable