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.

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

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

Open
DivineDominion opened this issue Jun 1, 2024 · 2 comments
Labels
good first issue Good for newcomers

Comments

@DivineDominion
Copy link
Contributor

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, "{^}!")
        }
    }
  • The loop should be in a block-based test helper for reuse
  • Test failure messages need to mention which buffer type failed to meet the condition
@ikelax
Copy link
Contributor

ikelax commented Jun 1, 2024

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 DivineDominion added the good first issue Good for newcomers label Jun 1, 2024
@DivineDominion
Copy link
Contributor Author

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants