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

Improve diffing of collections and structured values. #65

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

grynspan
Copy link
Contributor

@grynspan grynspan commented Oct 16, 2023

This PR improves the test output when comparing collections and structured values (using #expect()) when the comparison fails. Previously, we'd just the insertions and deletions as arrays, which was potentially hard to read and didn't show the test author where these changes occurred. Now, we convert the compared values into something similar to multi-line diff output. Consider the following test function:

@Test func f() {
  let lhs = """
  Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
  tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
  quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
  consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
  cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
  non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
  """
  let rhs = """
  Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
  tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
  quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
  potato salad. Duis aute irure dolor in reprehenderit in voluptate velit esse
  cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
  non dentistry, sunt in culpa qui officia deserunt mollit anim id est laborum.
  """
  #expect(lhs == rhs)
}

The output will now be:

◇ Test f() started.
✘ Test f() recorded an issue at MyTests.swift:45:5: Expectation failed: lhs == rhs
± 2 changes:
  "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod"
  "tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,"
  "quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo"
+ "potato salad. Duis aute irure dolor in reprehenderit in voluptate velit esse"
- "consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse"
  "cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat"
+ "non dentistry, sunt in culpa qui officia deserunt mollit anim id est laborum."
- "non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."
✘ Test f() failed after 0.001 seconds with 1 issue.

Similarly, with this test:

struct S: Equatable {
  var x: Int
  var y: String
}

@Test func f() {
  let lhs = S(x: 123, y: "abc")
  let rhs = S(x: 123, y: "def")
  #expect(lhs == rhs)
}

The output should appear similar to:

◇ Test f() started.
✘ Test f() recorded an issue at MyTests.swift:36:5: Expectation failed: lhs == rhs
± 1 change:
  MyTests.S(
    x: 123
-   y: "abc"
+   y: "def"
  )
✘ Test f() failed after 0.001 seconds with 1 issue.

Resolves rdar://66351980.

This PR improves the test output when comparing collections and structured values (using `#expect()`) when the comparison fails. Previously, we'd just the insertions and deletions as arrays, which was potentially hard to read and didn't show the test author _where_ these changes occurred. Now, we convert the compared values into something similar to multi-line `diff` output. Consider the following test function:

```swift
@test func f() {
  let lhs = """
  Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
  tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
  quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
  consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
  cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
  non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
  """
  let rhs = """
  Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
  tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
  quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
  potato salad. Duis aute irure dolor in reprehenderit in voluptate velit esse
  cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
  non dentistry, sunt in culpa qui officia deserunt mollit anim id est laborum.
  """
  #expect(lhs == rhs)
}
```

The output will now be:

```
◇ Test f() started.
✘ Test f() recorded an issue at MyTests.swift:45:5: Expectation failed: lhs == rhs
±  4 changes:
  "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod"
  "tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,"
  "quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo"
+ "potato salad. Duis aute irure dolor in reprehenderit in voluptate velit esse"
- "consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse"
  "cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat"
+ "non dentistry, sunt in culpa qui officia deserunt mollit anim id est laborum."
- "non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."
✘ Test f() failed after 0.001 seconds with 1 issue.
```

Similarly, with this test:

```swift
struct S: Equatable {
  var x: Int
  var y: String
}

@test func f() {
  let lhs = S(x: 123, y: "abc")
  let rhs = S(x: 123, y: "def")
  #expect(lhs == rhs)
}
```

The output should appear similar to:

```
◇ Test f() started.
✘ Test f() recorded an issue at MyTests.swift:36:5: Expectation failed: lhs == rhs
± 1 change:
  MyTests.S(
    x: 123
-   y: "abc"
+   y: "def"
  )
✘ Test f() failed after 0.001 seconds with 1 issue.
```

Resolves rdar://66351980.
@grynspan grynspan force-pushed the jgrynspan/66351980-better-diffs branch from cc77f69 to 43ba3c3 Compare October 16, 2023 23:10
@SeanROlszewski
Copy link
Contributor

So many thank yous are in order for you, @grynspan.

@SeanROlszewski
Copy link
Contributor

It's probably worth updating the docs for this change to mention that the diffing logic uses CustomStringConvertible and CustomDebugStringConvertible via String(describing:) under the hood, which can lead to unexpected diffs if the user does something unique or hard to diff.

@grynspan
Copy link
Contributor Author

Still working on the exact semantics of this change, since the exploded view of a value is not always the right view for comparison. But yes, we'll want to amend documentation at the same time.

…that types with specific needs can opt out of the default mirroring behaviour
@grynspan
Copy link
Contributor Author

@swift-ci please test

@grynspan
Copy link
Contributor Author

@swift-ci please test

.split(whereSeparator: \.isNewline)
.map { line in
switch line.first {
case "+":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking about refactoring this code a bit in the future to use SF Symbols when available, but I want to factor _Symbol out into its own file first, and that's not relevant to this PR.

@grynspan
Copy link
Contributor Author

@swift-ci please test

@grynspan grynspan added the enhancement New feature or request label Oct 25, 2023
@grynspan grynspan marked this pull request as draft February 13, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants