Skip to content

Conversation

@thomasvl
Copy link
Collaborator

@thomasvl thomasvl commented Nov 4, 2025

  • Have the Makefile provide better messaging in the conformance runners isn't found.
  • Have build.yml run the conformance tests as it is in the protobuf submodule.
  • Revise the cron based conformance run to be a protobuf HEAD run, so it happens on every PR and still happens on a cron as an early signal for when something might need attention.

@thomasvl thomasvl added the semver/none No version bump required. label Nov 4, 2025
@thomasvl thomasvl marked this pull request as ready for review November 4, 2025 15:18
@thomasvl thomasvl requested review from FranzBusch and Lukasa November 4, 2025 15:18
@thomasvl
Copy link
Collaborator Author

thomasvl commented Nov 4, 2025

@FranzBusch @Lukasa

The questions on this in my mind are:

  • For the submodule protobuf, it would be nice if that was a different job instead of a step as it would be parallel and thus faster for total results, but if we want that still on all the supported Xcode versions, we'd have to duplicate the matrix. We could do it just on the latest Swift release if we want the result and aren't worried about version specific conformance coverage. Any thoughts on changing that up?
  • Or, do we still want to drop the conformance tests with the submodule protobuf on CI or do we just do it on HEAD?

ps - we could also update the Makefile to do the build of the conformance runner so folks don't have to manually do that.

@thomasvl thomasvl force-pushed the test_head_conformance branch from 72efa6f to 95891c8 Compare November 4, 2025 15:46
@FranzBusch
Copy link
Member

The questions on this in my mind are:

  • For the submodule protobuf, it would be nice if that was a different job instead of a step as it would be parallel and thus faster for total results, but if we want that still on all the supported Xcode versions, we'd have to duplicate the matrix. We could do it just on the latest Swift release if we want the result and aren't worried about version specific conformance coverage. Any thoughts on changing that up?
  • Or, do we still want to drop the conformance tests with the submodule protobuf on CI or do we just do it on HEAD?

ps - we could also update the Makefile to do the build of the conformance runner so folks don't have to manually do that.

IMO it's fine if we run the conformance tests only with the latest Swift release.

@thomasvl
Copy link
Collaborator Author

thomasvl commented Nov 5, 2025

The questions on this in my mind are:

  • For the submodule protobuf, it would be nice if that was a different job instead of a step as it would be parallel and thus faster for total results, but if we want that still on all the supported Xcode versions, we'd have to duplicate the matrix. We could do it just on the latest Swift release if we want the result and aren't worried about version specific conformance coverage. Any thoughts on changing that up?
  • Or, do we still want to drop the conformance tests with the submodule protobuf on CI or do we just do it on HEAD?

ps - we could also update the Makefile to do the build of the conformance runner so folks don't have to manually do that.

IMO it's fine if we run the conformance tests only with the latest Swift release.

Amended.

Up to you all on if you want to make this new check required also.

@thomasvl thomasvl force-pushed the test_head_conformance branch from e7be361 to ad8f739 Compare November 5, 2025 15:06
@thomasvl
Copy link
Collaborator Author

thomasvl commented Nov 6, 2025

@Lukasa @FranzBusch ping

@thomasvl thomasvl requested a review from tbkka November 10, 2025 15:50
- Have the `Makefile` provide better messaging in the conformance runners isn't
  found.
- Have `build.yml` run the conformance tests as it is in the protobuf submodule.
- Revise the cron based conformance run to be a protobuf HEAD run, so it happens
  on every PR and still happens on a cron as an early signal for when something
  might need attention.
@thomasvl thomasvl force-pushed the test_head_conformance branch from ad8f739 to 8598595 Compare November 10, 2025 15:51
@thomasvl thomasvl requested a review from allevato November 11, 2025 13:57
@thomasvl thomasvl merged commit b2e8265 into apple:main Nov 11, 2025
13 checks passed
@thomasvl thomasvl deleted the test_head_conformance branch November 11, 2025 14:55
@thomasvl
Copy link
Collaborator Author

@FranzBusch @Lukasa I went ahead and landed this, can one of you make the new step required for PRs since we probably want to ensure the conformance tests are always passing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants