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.

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

Recognise and enable acceptance of AutoRecorded images. #72

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Recognise and enable acceptance of AutoRecorded images. #72

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6037f76
Recognise AutoRecord-created images.
babbage Apr 7, 2018
5e9274c
Enable acceptance of AutoRecorded images.
babbage Apr 7, 2018
4ca4504
deleteIfExists to delete file if it exists at URL.
babbage Apr 7, 2018
a32672a
Merge branch 'master' into AutoSnapshot
babbage Apr 18, 2018
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* Your contribution here.
* [#69](https://github.com/Antondomashnev/FBSnapshotsViewer/pull/69): Replace Swap with Accept and Accept All. - [@babbage](https://github.com/babbage).
* [#72](https://github.com/Antondomashnev/FBSnapshotsViewer/pull/72): Recognise and enable acceptance of AutoRecorded images. - [@babbage](https://github.com/babbage).

### 0.8.0 (11.10.2017)

Expand Down
8 changes: 7 additions & 1 deletion FBSnapshotsViewer/Extensions/FileManager+Move.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ import Foundation

extension FileManager {
func moveItem(at fromURL: URL, to toURL: URL) throws {
try self.removeItem(at: toURL)
try deleteIfExists(at: toURL)
try self.copyItem(at: fromURL, to: toURL)
}

func deleteIfExists(at url: URL) throws {
if fileExists(atPath: url.path) {
try self.removeItem(at: url)
}
}
}
2 changes: 1 addition & 1 deletion FBSnapshotsViewer/Managers/SnapshotTestResultAcceptor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class SnapshotTestResultAcceptor {
let failedImageSizeSuffix = imagePath.substring(with: failedImageSizeSuffixRange)
let recordedImageURLs = testResult.build.fbReferenceImageDirectoryURLs.flatMap { fbReferenceImageDirectoryURL -> URL? in
let url = fbReferenceImageDirectoryURL.appendingPathComponent(testResult.testClassName).appendingPathComponent("\(testResult.testName)\(failedImageSizeSuffix)").appendingPathExtension("png")
return fileManager.fileExists(atPath: url.path) ? url : nil
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need to change it?

Copy link
Collaborator Author

@babbage babbage Apr 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm. This change was the shortest path to implement the desired functionality here, which was to still create test images even if a reference image didn't exist. However, now that I look at it, it seems like maybe it would be better to still throw SnapshotTestResultAcceptorError.notExistedRecordedImage but then catch it and handle it by doing what we want with the autoRecording functionality. I think I was put off a bit by framing this as a SnapshotTestResultAcceptorError when really it's a SnapshotTestResultAcceptorResult, given a missing reference snapshot is no longer considered an error per se, if and when this pull request is integrated.

I will take another look at this.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks @babbage ;)

return url
}
guard let url = recordedImageURLs.first else {
throw SnapshotTestResultAcceptorError.notExistedRecordedImage(testResult: testResult)
Expand Down
1 change: 1 addition & 0 deletions FBSnapshotsViewer/Models/ApplicationLogLine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct ApplicationLogLineIndicatorContainer {
static func snapshotTestErrorMessageIndicators(for configuration: Configuration) -> [String] {
return [" : ((noErrors) is true) failed - Snapshot comparison failed",
" : failed - Snapshot comparison failed: Optional(Error Domain=FBSnapshotTestControllerErrorDomain Code=4",
" : failed - No previous reference image. New image has been stored for approval.",
" : failed - Test ran in record mode."]
}
}
Expand Down