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

Compile with -warnings-as-errors in CI (incl. StrictSendability) #281

Merged

Conversation

simonjbeaumont
Copy link
Collaborator

Motivation

We were not compiling with -warnings-as-errors in CI because some of the reference source code contained deprecation annotations. This meant that we are not getting CI failures for other warnings that we'd like to be clean of; specifically strict concurrency. However, in addition to the file-based reference tests, we now have in-memory snippet-based reference tests, which contain tests for deprecated OpenAPI schemas and operations.

Modifications

  • Remove deprecated tests from file-based reference test.
  • Clean up warnings under StrictConcurrency.
  • Compile with -warnings-as-errors in CI.
  • Opt into StrictConcurrency and -warnings-as-errors locally with environment variable.

Result

  • CI will catch sendability warning regressions.
  • Developing locally with SWIFT_OPENAPI_STRICT_CONCURRENCY will enable the same behaviour.

Test Plan

  • Added a canary commit to the end of this PR with a sendability issue, which will be reverted before merging.

@simonjbeaumont simonjbeaumont force-pushed the sb/strict-concurrency-enable branch from d975e48 to c23f760 Compare September 18, 2023 12:20
@simonjbeaumont
Copy link
Collaborator Author

simonjbeaumont commented Sep 18, 2023

Canary commit shows that the CI will help us catch the warnings1:

[737/761] Compiling OpenAPIGeneratorReferenceTests CompatabilityTest.swift
/code/Tests/OpenAPIGeneratorReferenceTests/CompatabilityTest.swift:213:42: error: capture of 'diagnosticsCollector' with non-sendable type 'RecordingDiagnosticCollector' in a `@Sendable` closure
                            diagnostics: diagnosticsCollector
                                         ^
/code/Tests/OpenAPIGeneratorReferenceTests/CompatabilityTest.swift:335:21: note: class 'RecordingDiagnosticCollector' does not conform to the 'Sendable' protocol
private final class RecordingDiagnosticCollector: DiagnosticCollector {
                    ^
...
error: fatalError
1

I'll now revert the canary commit for review.

Footnotes

  1. https://ci.swiftserver.group/job/swift-openapi-generator-swift59-prb/500/console

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

:shipit:

@simonjbeaumont simonjbeaumont merged commit a07ebd2 into apple:main Sep 18, 2023
@czechboy0 czechboy0 added the semver/none No version bump required. label Sep 18, 2023
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.

2 participants