Skip to content

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Sep 9, 2024

Motivation:

swift-testing has a number of advantages over XCTest (parameterisation, organisation, failure messages etc.), we should start using it instead of XCTest.

Modifications:

  • Convert the Status tests
  • Add a dependency on swift-testing on Linux (this can be removed when it's included as part of the toolchain)

Results:

Better tests

swift-testing has a number of advantages over XCTest (parameterisation,
organisation, failure messages etc.), we should start using it instead
of XCTest.

Modifications:

- Convert the Status tests
- Add a dependency on swift-testing on Linux (this can be removed when it's included as part of the toolchain)
- Fixed a couple of bugs found by additional testing

Results:

Better tests
@glbrntt glbrntt requested a review from gjcairo September 9, 2024 09:29
Copy link
Collaborator

@gjcairo gjcairo left a comment

Choose a reason for hiding this comment

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

LGTM, only left a small comment.
This is a lot nicer btw :D exciting times.

XCTAssertNil(Status.Code(rawValue: -1))
XCTAssertNil(Status.Code(rawValue: 1000))
XCTAssertNil(Status.Code(rawValue: .max))
@Test("Initialize from invalid rawValue", arguments: [-1, 17, 100, .max])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure it's really important, but we're not testing for all values from 17 to UInt8.max anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, not sure it really matters tbh. The UInt8 is an implementation detail so a bit odd to do all of those values given the parameter is an Int.

@glbrntt glbrntt enabled auto-merge (squash) September 9, 2024 11:56
@glbrntt glbrntt merged commit 17b2054 into grpc:main Sep 9, 2024
15 of 17 checks passed
@glbrntt glbrntt added the version/v2 Relates to v2 label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

version/v2 Relates to v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants