12 Mar 2022
Around 3 years ago I wrote a post showing my git-edit
command that helps me very quickly perform a rebase where I want to edit a specific commit in my history.
This script has served me well but the original version no longer works when trying to edit history whilst preserving merge commits (merge bubbles), which is the behaviour I want.
It’s a pretty small fix and is a classic case of me over thinking the original implementation and that resulting in it being more brittle than it needed to be.
tl;dr here’s the updated script
Quick recap
Given this history:
* c3b7967 (HEAD -> main) Add greet function
* b80d6e2 Add contributors list
* 38eccff Add readme
When I call my git-edit
command with the second commit:
The following will happen
- Git will perform a
git rebase --interactive b80d6e2~
- My
git-edit
script will be handed the file path of the git todo list and make the following change
- pick c3b7967 Add greet function
+ edit c3b7967 Add greet function
# Rebase b80d6e2..c3b7967 onto b80d6e2 (1 command)
#
# <git rebase instructions here>
#
- Now I make any edit I need and then run
git rebase --continue
to finish
Preserving merge bubbles - the problem
The issue with the above is that invoking git rebase
will flatten merge bubbles unless you pass the --rebase-merges
flag.
For example if I have this repo
* 5e327d2 (HEAD -> main) Merge branch 'feature/add-greet-function'
|\
| * 6c4ea2d (feature/add-greet-function) Add greet function
|/
* 9b18f79 Merge branch 'feature/add-contributors'
|\
| * e5d86e3 (feature/add-contributors) Add contributors list
|/
* 6716452 Merge branch 'feature/add-readme'
|\
| * a9754cb (feature/add-readme) Add readme
|/
* c3b7967
When I call git edit a9754cb
and make changes the result would be the following
* ab45054 (HEAD -> main) Add greet function
* 07fc0f8 Add contributors list
* a9754cb (feature/add-readme) Add readme
As I alluded to earlier my original implementation was actually more complicated than it needed to be and introduced brittleness - here it is:
#!/usr/bin/env ruby
if ENV['GIT_SEQUENCE_EDITOR'] != __FILE__
exec "GIT_SEQUENCE_EDITOR='#{__FILE__}' git rebase --interactive #{ARGV.first}~1"
end
todo_list = ARGV.first
lines = File.readlines(todo_list)
.lazy
.take_while { |line| line != "\n" }
.map { |line| line.gsub("fixup", "pick") }
.to_a
lines[0] = lines[0].gsub("pick", "edit")
File.write(todo_list, lines.join)
There are two things to call out in this implementation that went wrong.
1) I have hardcoded the assumption that the line I need to edit is the first in the file
2) I’m iterating over the todo list line by line and stopping on the first empty new line
If we look at the todo list for the previous repo when adding the --preserve-merges
flag we can see that both points above are troublesome
label onto
# Branch feature-add-readme
reset onto
pick a9754cb Add readme
label feature-add-readme
# Branch feature-add-contributors
reset onto
merge -C 6716452 feature-add-readme # Merge branch 'feature/add-readme'
label branch-point
pick e5d86e3 Add contributors list
label feature-add-contributors
# Branch feature-add-greet-function
reset branch-point # Merge branch 'feature/add-readme'
merge -C 9b18f79 feature-add-contributors # Merge branch 'feature/add-contributors'
label branch-point-2
pick 6c4ea2d Add greet function
label feature-add-greet-function
reset branch-point-2 # Merge branch 'feature/add-contributors'
merge -C 5e327d2 feature-add-greet-function # Merge branch 'feature/add-greet-function'
# Rebase c3b7967..5e327d2 onto c3b7967 (16 commands)
#
# <git rebase instructions here>
#
The format is completely different to the simple example above because git needs more details to know how to rebuild history.
The new format includes empty lines in a few places and the line to edit is no longer the first line 🤦🏼.
Preserving merge bubbles - the fix
The simplest fix is to be less strict - instead of treating this as a line by line operation like a person would I can just be fairly slapdash with substitutions to get the same result.
#!/usr/bin/env ruby
if ENV['GIT_SEQUENCE_EDITOR'] != __FILE__
exec "GIT_SEQUENCE_EDITOR='#{__FILE__}' TARGET_SHA=#{ARGV.first} git rebase -i #{ARGV.first}~1 --rebase-merges"
end
sha = ENV["TARGET_SHA"]
todo_list_file_name = ARGV.first
transformed_todo_list = File.read(todo_list_file_name)
.gsub(/^fixup/, "pick")
.gsub(/^pick #{sha}/, "edit #{sha}")
File.write(todo_list_file_name, transformed_todo_list)
In the above I don’t read the lines of the file I just YOLO it and swap any fixup
at the beginning of a line with pick
and more importantly I change any pick
followed by my sha to an edit
.
The other subtle change is that the when my script is handed the todo list I don’t know what sha the command was invoked with.
To get around this when I invoke the rebase I first set an environment variable TARGET_SHA
so I can pull it out when updating the todo list.
With these changes in place my git-edit
command now works for the simple case and in the less common case where I’m editing further back in history with merge commits.
Bonus
When changing history I have two common cases
1) Making edits to source
2) Updating a commit message
The mechanism for doing either of these is identical apart from needing to use the todo list command reword
instead of edit
.
To avoid writing an identical script I instead create a single script called git-edit
and then symlink to it with another file called git-reword
.
I can then tweak my script to look like this
#!/usr/bin/env ruby
if ENV['GIT_SEQUENCE_EDITOR'] != __FILE__
exec "GIT_SEQUENCE_EDITOR='#{__FILE__}' TARGET_SHA=#{ARGV.first} git rebase -i #{ARGV.first}~1 --rebase-merges"
end
command = File.basename($0).split('-').last
sha = ENV["TARGET_SHA"]
todo_list_file_name = ARGV.first
transformed_todo_list = File.read(todo_list_file_name)
.gsub(/^fixup/, "pick")
.gsub(/^pick #{sha}/, "#{command} #{sha}")
File.write(todo_list_file_name, transformed_todo_list)
In the above the command is determined by which git command was invoked e.g.
Calling git edit
invokes the git-edit
script and calling git reword
invokes the git-reword
script.
The rebase command is then extracted from the script’s file name using
command = File.basename($0).split('-').last
18 May 2021
Most of us have to deal with integrating 3rd party APIs into our apps for a multitude of reasons.
Wisdom tells us to wrap dependencies and make them testable but a lot of vendors unintentionally make this really difficult.
In this post we are going to look at an API that has several design flaws that make it awkward to isolate and then we will run through the various techniques to tame the dependency.
Let’s start by looking at a stripped down example that shows a few of the issues
public class ThirdParty {
public static var delegate: ThirdPartyDelegate?
public static func loadFeedbackForm(_ id: String) { /* ... */ }
}
public protocol ThirdPartyDelegate {
func formDidload(_ viewController: UIViewController)
func formDidFailLoading(error: ThirdPartyError)
}
public struct ThirdPartyError: Error {
public let description: String
}
The design decisions that the third party author has made make this tricky to work with for a few reasons:
delegate
and loadFeedbackForm
are both static
making this a big ol’ blob of global state
- It’s using a delegate pattern, which is a bit old school so we’ll want to convert to a callback based API
ThirdPartyError
has no public init
so we can’t create an instance our self for our tests
- The delegate callbacks don’t give any indication of what ID they are being invoked for
Figuring out the high level plan
We don’t want our application to use ThirdParty
directly as it ties us to the 3rd party API design and we can’t really test effectively.
What we can do is create a protocol for how we want the API to look and then make a concrete implementation of this protocol for wrapping the dependency.
The high level view will look like this:
A reasonable API might look like this:
protocol FeedbackService {
func load(_ id: String, completion: @escaping (Result<UIViewController, Error>) -> Void) throws
}
With this set up our tests can verify the App behaviour by substituting a test double that implements FeedbackService
so that we don’t hit the real ThirdParty
implementation.
These tests are not particularly interesting to look at as creating a class MockFeedbackService: FeedbackService
is fairly easy.
However it is interesting to delve in to how to wrap and verify the ThirdParty
dependency itself with all its design warts.
Wrapping the dependency
The first design pain is that delegate
is a static var
and the delegate callbacks do not return any identifier to differentiate multiple loadFeedbackForm
calls.
From this we can take the decision to make it an error to have more than one loadFeedbackForm
call in flight at any one time.
We can use the fact that if the delegate
is non-nil
then we must already have an inflight request.
To write a test to cover this we can create an instance of ThirdPartyFeedbackService
and provide a stubbed function for fetching the delegate, the production code (see below the test) will default this function to proxying to ThirdParty.delegate
.
Hardcoding this getDelegate
function to return a non-nil
value means that any subsequent call to load should throw an exception:
func testWhenLoadIsInFlight_subsequentLoadsWillThrow() throws {
let subject = ThirdPartyFeedbackService(getDelegate: { ThirdPartyFeedbackService() })
XCTAssertThrowsError(try subject.load("test", completion: { _ in })) { error in
XCTAssertTrue(error is ThirdPartyFeedbackService.MultipleFormLoadError)
}
}
The production code to make this pass would look like this
public class ThirdPartyFeedbackService: FeedbackService {
private let getDelegate: () -> ThirdPartyDelegate?
public init(getDelegate: @escaping () -> ThirdPartyDelegate? = { ThirdParty.delegate }) {
self.getDelegate = getDelegate
}
public func load(_ id: String, completion: @escaping (Result<UIViewController, Error>) -> Void) throws {
guard getDelegate() == nil else {
throw MultipleFormLoadError()
}
}
public struct MultipleFormLoadError: Error {}
}
extension ThirdPartyFeedbackService: ThirdPartyDelegate {
public func formDidload(_ viewController: UIViewController) {}
public func formDidFailLoading(error: ThirdPartyError) {}
}
The next thing to verify is that a normal load call correctly sets the delegate
and also that we pass along the right id to the underlying ThirdParty
dependency.
Again we don’t want to actually invoke ThirdParty
so we need to provide stubs for loadFeedback
and setDelegate
that would be set to the production in normal usage.
A test to exercise this might look like this:
func testLoadSetsDelegateAndInvokesLoad() throws {
// Given
var capturedFormID: String?
var capturedDelegate: ThirdPartyDelegate?
let subject = ThirdPartyFeedbackService(
loadFeedback: { capturedFormID = $0 },
getDelegate: { nil },
setDelegate: { capturedDelegate = $0 }
)
// When
try subject.load("some-id") { _ in }
// Then
XCTAssertEqual("some-id", capturedFormID)
XCTAssertTrue(subject === capturedDelegate)
}
To make this pass we can update the production code like this:
public class ThirdPartyFeedbackService: FeedbackService {
+ private let loadFeedback: (String) -> Void
private let getDelegate: () -> ThirdPartyDelegate?
+ private let setDelegate: (ThirdPartyDelegate?) -> Void
public init(
+ loadFeedback: @escaping (String) -> () = ThirdParty.loadFeedbackForm,
getDelegate: @escaping () -> ThirdPartyDelegate? = { ThirdParty.delegate },
+ setDelegate: @escaping (ThirdPartyDelegate?) -> Void = { ThirdParty.delegate = $0 }
) {
+ self.loadFeedback = loadFeedback
self.getDelegate = getDelegate
+ self.setDelegate = setDelegate
}
public func load(_ id: String, completion: @escaping (Result<UIViewController, Error>) -> Void) throws {
guard getDelegate() == nil else {
throw MultipleFormLoadError()
}
+ setDelegate(self)
+ loadFeedback(id)
}
public struct MultipleFormLoadError: Error {}
}
extension ThirdPartyFeedbackService: ThirdPartyDelegate {
public func formDidload(_ viewController: UIViewController) {}
public func formDidFailLoading(error: ThirdPartyError) {}
}
With the above modifications we know we are setting things up correctly so the next step is to verify that a successful load will invoke our completion handler.
This test will exercise this for us:
func testWhenLoadIsSuccessful_itInvokesTheCompletionWithTheLoadedViewController() throws {
// Given
var capturedDelegate: ThirdPartyDelegate?
let subject = ThirdPartyFeedbackService(
loadFeedback: { _ in },
getDelegate: { nil },
setDelegate: { capturedDelegate = $0 }
)
let viewControllerToPresent = UIViewController()
var capturedViewController: UIViewController?
try subject.load("some-id") { result in
if case let .success(viewController) = result {
capturedViewController = viewController
}
}
// When
subject.formDidload(viewControllerToPresent)
// Then
XCTAssertEqual(viewControllerToPresent, capturedViewController)
XCTAssertNil(capturedDelegate)
}
This change makes the test pass:
public class ThirdPartyFeedbackService: FeedbackService {
+ private var completion: ((Result<UIViewController, Error>) -> Void)?
private let loadFeedback: (String) -> Void
private let getDelegate: () -> ThirdPartyDelegate?
private let setDelegate: (ThirdPartyDelegate?) -> Void
public init(
loadFeedback: @escaping (String) -> () = ThirdParty.loadFeedbackForm,
getDelegate: @escaping () -> ThirdPartyDelegate? = { ThirdParty.delegate },
setDelegate: @escaping (ThirdPartyDelegate?) -> Void = { ThirdParty.delegate = $0 }
) {
self.loadFeedback = loadFeedback
self.getDelegate = getDelegate
self.setDelegate = setDelegate
}
public func load(_ id: String, completion: @escaping (Result<UIViewController, Error>) -> Void) throws {
guard getDelegate() == nil else {
throw MultipleFormLoadError()
}
+ self.completion = { [setDelegate] result in
+ setDelegate(nil)
+ completion(result)
+ }
setDelegate(self)
loadFeedback(id)
}
public struct MultipleFormLoadError: Error {}
}
extension ThirdPartyFeedbackService: ThirdPartyDelegate {
public func formDidload(_ viewController: UIViewController) {
+ completion?(.success(viewController))
}
public func formDidFailLoading(error: ThirdPartyError) {}
}
Finally we need to handle the failing case:
func testWhenLoadFails_itInvokesTheCompletionWithAnError() throws {
// Given
var capturedDelegate: ThirdPartyDelegate?
let subject = ThirdPartyFeedbackService(loadFeedback: { _ in }, getDelegate: { nil }, setDelegate: { capturedDelegate = $0 })
let errorToPresent = NSError(domain: "test", code: 999, userInfo: nil)
var capturedError: NSError?
try subject.load("some-id") { result in
if case let .failure(error) = result {
capturedError = error as NSError
}
}
// When
subject.formDidFailLoading(error: NSError(domain: "test", code: 999, userInfo: nil))
// Then
XCTAssertEqual(errorToPresent, capturedError)
XCTAssertNil(capturedDelegate)
}
This requires a fairly complex change because we can’t create a ThirdPartyError
because the init
is not public
.
Instead we need to work around this by making our function generic so that the compiler will write one implementation that works for ThirdPartyError
types and one implementation that works with the Error
type we provide in our tests.
public class ThirdPartyFeedbackService: FeedbackService {
private var completion: ((Result<UIViewController, Error>) -> Void)?
private let loadFeedback: (String) -> Void
private let getDelegate: () -> ThirdPartyDelegate?
private let setDelegate: (ThirdPartyDelegate?) -> Void
public init(
loadFeedback: @escaping (String) -> () = ThirdParty.loadFeedbackForm,
getDelegate: @escaping () -> ThirdPartyDelegate? = { ThirdParty.delegate },
setDelegate: @escaping (ThirdPartyDelegate?) -> Void = { ThirdParty.delegate = $0 }
) {
self.loadFeedback = loadFeedback
self.getDelegate = getDelegate
self.setDelegate = setDelegate
}
public func load(_ id: String, completion: @escaping (Result<UIViewController, Error>) -> Void) throws {
guard getDelegate() == nil else {
throw MultipleFormLoadError()
}
self.completion = { [setDelegate] result in
setDelegate(nil)
completion(result)
}
setDelegate(self)
loadFeedback(id)
}
public struct MultipleFormLoadError: Error {}
}
extension ThirdPartyFeedbackService: ThirdPartyDelegate {
public func formDidload(_ viewController: UIViewController) {
completion?(.success(viewController))
}
- public func formDidFailLoading(error: ThirdPartyError) {
+ public func formDidFailLoading<T: Error>(error: T) {
+ completion?(.failure(error))
}
}
How’d we do?
The design issues raised at the beginning made it harder to wrap the dependency and we had to make some decisions along the way like how we fail if multiple loads are called.
We had to get a little creative to make this work and things like thread safety and cancellation are all things omitted for brevity.
Just for kicks
We might look at our protocol and think that it’s not really a useful abstraction to have.
Migrating to using a simple value type is fairly mechanical and worth having a look to see how it pans out.
public struct FeedbackService {
public let load: (_ form: String, _ completion: @escaping (Result<UIViewController, Error>) -> Void) throws -> Void
}
extension FeedbackService {
public static func thirdParty(
loadFeedback: @escaping (String) -> () = ThirdParty.loadFeedbackForm,
getDelegate: @escaping () -> ThirdPartyDelegate? = { ThirdParty.delegate },
setDelegate: @escaping (ThirdPartyDelegate?) -> Void = { ThirdParty.delegate = $0 },
makeDelegate: () -> Delegate = Delegate.init
) -> FeedbackService {
let delegate = makeDelegate()
return .init { id, completion in
guard getDelegate() == nil else {
throw MultipleFormLoadError()
}
delegate.completion = { result in
setDelegate(nil)
completion(result)
}
setDelegate(delegate)
loadFeedback(id)
}
}
public class Delegate: ThirdPartyDelegate {
var completion: ((Result<UIViewController, Error>) -> Void)?
public init() {}
public func formDidload(_ viewController: UIViewController) {
completion?(.success(viewController))
}
public func formDidFailLoading<T: Error>(error: T) {
completion?(.failure(error))
}
}
public struct MultipleFormLoadError: Error {}
}
Looking at the above - the core of the logic does not change at all.
We lose some lines as we are capturing the params to the thirdParty
function and don’t need to create instance variables.
We gain some lines by implementing an inline delegate class.
The tests can also be updated with minimal changes:
final class ThirdPartyFeedbackServiceTests: XCTestCase {
func testLoadSetsDelegateAndInvokesLoad() throws {
// Given
let delegate = FeedbackService.Delegate()
var capturedFormID: String?
var capturedDelegate: ThirdPartyDelegate?
let subject = FeedbackService.thirdParty(
loadFeedback: { capturedFormID = $0 },
getDelegate: { nil },
setDelegate: { capturedDelegate = $0 },
makeDelegate: { delegate }
)
// When
try subject.load("some-id") { _ in }
// Then
XCTAssertEqual("some-id", capturedFormID)
XCTAssertTrue(delegate === capturedDelegate)
}
func testWhenLoadIsInFlight_subsequentLoadsWillThrow() throws {
let subject = FeedbackService.thirdParty(getDelegate: { FeedbackService.Delegate() })
XCTAssertThrowsError(try subject.load("test", { _ in })) { error in
XCTAssertTrue(error is FeedbackService.MultipleFormLoadError)
}
}
func testWhenLoadIsSuccessful_itInvokesTheCompletionWithTheLoadedViewController() throws {
// Given
let delegate = FeedbackService.Delegate()
var capturedDelegate: ThirdPartyDelegate?
let subject = FeedbackService.thirdParty(loadFeedback: { _ in }, getDelegate: { nil }, setDelegate: { capturedDelegate = $0 }, makeDelegate: { delegate })
let viewControllerToPresent = UIViewController()
var capturedViewController: UIViewController?
try subject.load("some-id") { result in
if case let .success(viewController) = result {
capturedViewController = viewController
}
}
// When
delegate.formDidload(viewControllerToPresent)
// Then
XCTAssertEqual(viewControllerToPresent, capturedViewController)
XCTAssertNil(capturedDelegate)
}
func testWhenLoadFails_itInvokesTheCompletionWithAnError() throws {
// Given
let delegate = FeedbackService.Delegate()
var capturedDelegate: ThirdPartyDelegate?
let subject = FeedbackService.thirdParty(loadFeedback: { _ in }, getDelegate: { nil }, setDelegate: { capturedDelegate = $0 }, makeDelegate: { delegate })
let errorToPresent = NSError(domain: "test", code: 999, userInfo: nil)
var capturedError: NSError?
try subject.load("some-id") { result in
if case let .failure(error) = result {
capturedError = error as NSError
}
}
// When
delegate.formDidFailLoading(error: NSError(domain: "test", code: 999, userInfo: nil))
// Then
XCTAssertEqual(errorToPresent, capturedError)
XCTAssertNil(capturedDelegate)
}
}
Conclusion
I quite like the second version and think it was worth taking the extra time to explore both approaches even if it boils down to personal preference.
It would be nice if third party vendors made testing a first class concern rather than leaving us scratching our heads for work arounds or just accepting that we will call the live code.
Hopefully some of the techniques/ideas are useful and no doubt I’ll find myself reading this post in a few months times when I have to wrap something else awkward.
16 May 2021
Have you ever needed to automate a task that needs to be run on an iOS simulator and not known quite how to make it work in an unsupervised way?
As an example - imagine we need to take some JSON files, run each one of them through an app and capture the generated UI so that the screenshots can be uploaded for further processing.
At a high level we want an executable that can take a directory full of input, run a task inside the simulator and place the output of all of the tasks into a different directory.
We’ll start by looking at building something to run our task inside a simulator and then see how to get data in/out of the process.
Running our task
To run our code inside the simulator we can make use of a new test suite with a single test method.
The test method will enumerate files found at the input directory path, execute some code on the simulator and store the results in a different directory.
Here’s a basic implementation of the above:
import XCTest
class Processor: XCTestCase {
func testExample() throws {
let (inputDirectory, outputDirectory) = try readDirectoryURLs()
let fileManager = FileManager.default
try fileManager.createDirectory(at: outputDirectory, withIntermediateDirectories: true)
try fileManager.contentsOfDirectory(at: inputDirectory, includingPropertiesForKeys: nil).forEach {
try eval($0).write(to: outputDirectory.appendingPathComponent($0.lastPathComponent))
}
}
private func readDirectoryURLs() throws -> (input: URL, output: URL) {
func read(_ path: String) throws -> URL {
URL(fileURLWithPath: try String(contentsOf: Bundle(for: Processor.self).bundleURL.appendingPathComponent("\(path)-directory-path")).trimmingCharacters(in: .whitespacesAndNewlines))
}
return try (read("input"), read("output"))
}
}
The interesting things to note above are:
- The input/output directory paths need to be written to files called
input-directory-path
and output-directory-path
inside the test bundle
- The
eval
function is a function that can read the contents of a file and return a result that we can write to the output directory - this is where all of the real work would happen
There are plenty of things that could be done to customise the above for individual use cases but it’s enough for this post.
How do we set up the input-directory-path
and output-directory-path
files inside the test bundle?
Wrapping the task
In order to inject the relevant paths we need to ensure that our test suite is built and then run as two separate steps.
This gives us a chance to build the project, inject our file paths and then actually execute the test.
A Ruby script to do this would look something like the following:
#!/usr/bin/env ruby
unless ARGV.count == 2
puts "USAGE: #{$PROGRAM_NAME} INPUT_DIRECTORY OUTPUT_DIRECTORY"
exit 1
end
input_directory, output_directory = *ARGV
def xcode_build mode
`xcodebuild #{mode} -scheme Processor -destination 'platform=iOS Simulator,name=iPhone 12,OS=14.5' -derivedDataPath ./.build`
end
xcode_build "build-for-testing"
Dir[".build/**/Processor.app"].each do |path|
write = -> name, contents do
File.open("#{path}/PlugIns/ProcessorTests.xctest/#{name}-directory-path", 'w') do |file| file.puts contents end
end
write["input", input_directory]
write["output", output_directory]
end
xcode_build "test-without-building"
This script is doing the following:
- Basic input validation to ensure that both an input and output path have been provided
- Run
xcodebuild
with the action of build-for-testing
to ensure that the test suite is built and not run
- Write the
input-directory-path
and output-directory-path
files into the test bundle
- Run
xcodebuild
with the action of test-without-building
to execute the test suite
With all of these pieces in place and assuming we named this script run-processor
we can execute this script like this:
./run-processor /path/to/input /path/to/output
Result
We have a pretty bare bones implementation that should demonstrate the general idea and leave plenty of scope for expansion and experimentation.