CleanCocoa / DeclarativeTextKit

Swift DSL to perform mutations on e.g. NSTextViews in a declarative style.
MIT License
42 stars 1 forks source link

Add shared testing harness for all Buffer-conforming types we ship #6

Open DivineDominion opened 4 months ago

DivineDominion commented 4 months ago

E.g. NSTextViewBuffer, MutableStringBuffer, Undoable<Buffer> should all behave the same.

I went with code duplication for each of these so far, but that's getting a bit tedious. I notice that I'm feeling a resistance towards adding new shared Buffer features or changing the API because I need to touch 3+ test suites.

A shared suite would be cool. As a matter of fact, I have never used something like that with XCTest, though.

A naive approach would be to merge all test files into 1, and then add a helper that transforms this:

    func testDelete() throws {
        let buffer = MutableStringBuffer("Hello: world!")
        buffer.insertionLocation = length(of: "Hello: wor")

        assertBufferState(buffer, "Hello: wor{^}ld!")

        try buffer.delete(in: .init(location: 0, length: 4))
        assertBufferState(buffer, "o: wor{^}ld!")

        try buffer.delete(in: .init(location: 0, length: 4))
        assertBufferState(buffer, "or{^}ld!")

        try buffer.delete(in: .init(location: 0, length: 4))
        assertBufferState(buffer, "{^}!")
    }

... into something like this:

    func testDelete() throws {
        let originalContent = "Hello: world!"
        foreach buffer in [
            MutableStringBuffer(originalContent), 
            textView(originalContent),
            Undoable(MutableStringBuffer(originalContent)),
        ] { 
          buffer.insertionLocation = length(of: "Hello: wor")

          assertBufferState(buffer, "Hello: wor{^}ld!")

          try buffer.delete(in: .init(location: 0, length: 4))
          assertBufferState(buffer, "o: wor{^}ld!")

          try buffer.delete(in: .init(location: 0, length: 4))
          assertBufferState(buffer, "or{^}ld!")

          try buffer.delete(in: .init(location: 0, length: 4))
          assertBufferState(buffer, "{^}!")
        }
    }
ikelax commented 4 months ago

Test contexts implemented with XCTContext may be useful here.

@DivineDominion Is that also a good first issue? The scope of this seems to be quite narrow.

DivineDominion commented 4 months ago

Oh yeah, that could work indeed for non-UI tests!

Changing that should be simple enough because it's a mechanical change where you'd mostly need to make sure the common tests cover the most ground (in case some of the tests have more assertions than others).

Note: NSTextViewBuffer may behave differently w.r.t. maintaining the user's selected range after inserting/deleting/replacing text. I've adapted some parts, but I expect some differences there when merging tests.

My recommendation to approach this would be to start without breaking anything, additively, with the expectation that the tests all stay green (apart from the note above), e.g.

  1. Add a BufferTests or BufferConformanceTests class,
  2. Copy all assertions on common Buffer methods from e.g. MutableStringBuffer (which should cover most of the ground)
  3. Check the other buffer test suites (classes) whether they test more or other interesting details in their test cases (functions) and copy missing pieces as needed to have the most assertions in the shared test suite.
  4. Try to change the behavior of the implementation(s) to break the tests to verify they run at all :) If in doubt, comment-out all method bodies of all Buffer-related functions to make all tests fail. This ensures that the shared tests actually do something and aren't accidentally no-ops. (It's not too uncommon :) I ran into that situation in this project, too, at some point, where a test succeeded only because I called the wrong function that didn't change anything)