Skip to content

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Sep 11, 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 MethodConfig coding tests
  • Fixed a couple of bugs found by additional testing

Results:

Fewer bugs, better tests

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 MethodConfig coding tests
- Fixed a couple of bugs found by additional testing

Results:

Fewer bugs, better tests
@glbrntt glbrntt requested a review from gjcairo September 11, 2024 15:31
@glbrntt glbrntt added the version/v2 Relates to v2 label Sep 11, 2024
@glbrntt glbrntt enabled auto-merge (squash) September 11, 2024 15:31
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.

Overall this looks good! I do wonder though whether having input files instead of the JSON in-line at each test case makes for tidier or harder to read test cases (as you'll have to move back and forth). Any reason in particular you moved them to input files?

@glbrntt
Copy link
Collaborator Author

glbrntt commented Sep 18, 2024

Overall this looks good! I do wonder though whether having input files instead of the JSON in-line at each test case makes for tidier or harder to read test cases (as you'll have to move back and forth). Any reason in particular you moved them to input files?

Yeah, so I only moved some of them. Inline is easier to follow if it's small. Once they get more than a handful of lines I found them a but unwieldy and writing them inline is a bit more a pain and error prone (you need to make sure you escape everything correctly etc.)

@glbrntt glbrntt requested a review from gjcairo September 18, 2024 14:01
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.

That makes sense - thanks George!

@glbrntt glbrntt merged commit 07123ed into grpc:main Sep 18, 2024
14 of 17 checks passed
@glbrntt glbrntt deleted the v2/swift-testing-method-config-coding branch September 24, 2024 19:10
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