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

ci: Finish migration to Github Actions #604

Merged
merged 17 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 68 additions & 9 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,71 @@
name: Pull Request
name: PR

on:
pull_request:
types: [opened, reopened, synchronize, ready_for_review]
pull_request:
types: [opened, reopened, synchronize]

jobs:
call-reusable-pull-request-workflow:
name: Checks
uses: apple/swift-nio/.github/workflows/reusable_pull_request.yml@main
with:
benchmarks_linux_enabled: false
soundness:
name: Soundness
uses: apple/swift-nio/.github/workflows/soundness.yml@main
with:
api_breakage_check_enabled: true
broken_symlink_check_enabled: true
docs_check_enabled: true
format_check_enabled: true
license_header_check_enabled: true
license_header_check_project_name: "SwiftOpenAPIGenerator"
shell_check_enabled: true
unacceptable_language_check_enabled: true
Comment on lines +12 to +19
Copy link
Member Author

Choose a reason for hiding this comment

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

Why are we enabling them explicitly? The default is true

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it's more visible. The fact that some are default to true and some to false is confusing IMO. I like to be able to see what tests we're running in the YAML.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am fine with explicitly stating but just to be clear no check is defaulting to false.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, my mistake. Thanks for confirming :)


unit-tests:
name: Unit tests
uses: apple/swift-nio/.github/workflows/unit_tests.yml@main
with:
linux_5_8_enabled: false
linux_5_9_arguments_override: "--explicit-target-dependency-import-check error"
linux_5_10_arguments_override: "--explicit-target-dependency-import-check error"
Comment on lines +26 to +27
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we track enabling warnings as errors for those pipelines at some point?

Copy link
Contributor

Choose a reason for hiding this comment

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

More than happy to. But that's over-and-above what we currently had in this project. We can track it separately.

linux_nightly_6_0_arguments_override: "--explicit-target-dependency-import-check error"
linux_nightly_main_enabled: false

integration-test:
name: Integration test
uses: apple/swift-nio/.github/workflows/swift_matrix.yml@main
with:
name: "Integration test"
matrix_linux_command: "apt-get update -yq && apt-get install -yq jq && ./scripts/run-integration-test.sh"
matrix_linux_5_8_enabled: false
matrix_linux_nightly_main_enabled: false
Copy link
Member Author

Choose a reason for hiding this comment

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

Why not run them on the nightly main?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it's materially slower and not blocking a PR. Note that this PR does use the nightly Swift versions in the scheduled workflow so we'll still be alerted. I'd really like to only have PR pipelines that are gating PR merges, especially when we have external contributors.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am surprised. Nightly main is slower than nightly 6.0? If so we should report that since this hints at a compiler speed regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

No no. It's slower because it's far less likely to have the image available.

What's really annoying (but I realise a Github Action limitation) is that this will still do a bunch of the warmup anyway because the if: false is only at the step-level.

I have some ideas on how we can make that better though, if you're open to them.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have some ideas here as well. We could dynamically generate the matrix instead of having it hard coded like now.


compatibility-test:
name: Compatibility test
runs-on: ubuntu-latest
container:
image: swift:latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Run OpenAPI document compatibilty test
env:
SWIFT_OPENAPI_COMPATIBILITY_TEST_ENABLE: "true"
SWIFT_OPENAPI_COMPATIBILITY_TEST_SKIP_BUILD: "true"
SWIFT_OPENAPI_COMPATIBILITY_TEST_FILTER: OpenAPIGeneratorReferenceTests.CompatibilityTest
SWIFT_OPENAPI_COMPATIBILITY_TEST_PARALLEL_CODEGEN: "true"
SWIFT_OPENAPI_COMPATIBILITY_TEST_NUM_BUILD_JOBS: 1
run: swift test --filter ${SWIFT_OPENAPI_COMPATIBILITY_TEST_FILTER}

example-packages:
name: Example packages
uses: apple/swift-nio/.github/workflows/swift_matrix.yml@main
simonjbeaumont marked this conversation as resolved.
Show resolved Hide resolved
with:
name: "Example packages"
matrix_linux_command: "./scripts/test-examples.sh"
matrix_linux_5_8_enabled: false
matrix_linux_nightly_main_enabled: false
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here. Why not nightly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Answered above.


swift-6-language-mode:
name: Swift 6 Language Mode
uses: apple/swift-nio/.github/workflows/swift_6_language_mode.yml@main
simonjbeaumont marked this conversation as resolved.
Show resolved Hide resolved
if: false # Disabled for now.
32 changes: 32 additions & 0 deletions .github/workflows/scheduled.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Scheduled

on:
schedule:
- cron: "0 8,20 * * *"

jobs:
unit-tests:
name: Unit tests
uses: apple/swift-nio/.github/workflows/unit_tests.yml@main
with:
linux_5_8_enabled: false
linux_5_9_arguments_override: "--explicit-target-dependency-import-check error"
linux_5_10_arguments_override: "--explicit-target-dependency-import-check error"
linux_nightly_6_0_arguments_override: "--explicit-target-dependency-import-check error"
linux_nightly_main_arguments_override: "--explicit-target-dependency-import-check error"

integration-test:
name: Integration test
uses: apple/swift-nio/.github/workflows/swift_matrix.yml@main
with:
name: "Integration test"
matrix_linux_command: "apt-get update -yq && apt-get install -yq jq && ./scripts/run-integration-test.sh"
matrix_linux_5_8_enabled: false

example-packages:
name: Example packages
uses: apple/swift-nio/.github/workflows/swift_matrix.yml@main
with:
name: "Example packages"
matrix_linux_command: "./scripts/test-examples.sh"
matrix_linux_5_8_enabled: false
41 changes: 41 additions & 0 deletions .licenseignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
.gitignore
.licenseignore
.swiftformatignore
.spi.yml
.swift-format
.github/*
CODE_OF_CONDUCT.md
CONTRIBUTING.md
CONTRIBUTORS.txt
LICENSE.txt
NOTICE.txt
Package.swift
Package.resolved
README.md
SECURITY.md
scripts/unacceptable-language.txt
Tests/PetstoreConsumerTests/Generated
Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/*
docker/*
**/*.docc/*
**/.gitignore
**/Package.swift
**/Package.resolved
**/README.md
**/openapi.yaml
**/malformed-openapi.yaml
**/openapi.yml
**/petstore.yaml
**/openapi-generator-config.yaml
**/openapi-generator-config.yml
**/docker-compose.yaml
**/docker/*
**/.dockerignore
Plugins/OpenAPIGenerator/PluginsShared
Plugins/OpenAPIGeneratorCommand/PluginsShared
Examples/HelloWorldiOSClientAppExample/HelloWorldiOSClientApp.*
Examples/HelloWorldiOSClientAppExample/HelloWorldiOSClientApp/Assets.xcassets/*
Examples/HelloWorldiOSClientAppExample/HelloWorldiOSClientApp/Preview*
Examples/**/Generated*
**/Makefile
**/*.html
4 changes: 4 additions & 0 deletions .swiftformatignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Tests/OpenAPIGeneratorReferenceTests/Resources
Sources/swift-openapi-generator/Documentation.docc
Examples/**/Generated/*
Examples/**/GeneratedSources/*
37 changes: 32 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,40 @@ A good patch is:
3. Documented, adding API documentation as needed to cover new functions and properties.
4. Accompanied by a great commit message, using our commit message template.

### Run `./scripts/soundness.sh`
### Run CI checks locally

The scripts directory contains a [soundness.sh script](https://github.com/apple/swift-openapi-generator/blob/main/scripts/soundness.sh)
that enforces additional checks, like license headers and formatting style.
You can run the Github Actions workflows locally using
[act](https://github.com/nektos/act). To run all the jobs that run on a pull
request, use the following command:

Please make sure to `./scripts/soundness.sh` before pushing a change upstream, otherwise it is likely the PR validation will fail
on minor changes such as a missing `self.` or similar formatting issues.
```
% act pull_request
```

To run just a single job, use `workflow_call -j <job>`, and specify the inputs
the job expects. For example, to run just shellcheck:

```
% act workflow_call -j soundness --input shell_check_enabled=true
```

To bind-mount the working directory to the container, rather than a copy, use
`--bind`. For example, to run just the formatting, and have the results
reflected in your working directory:

```
% act --bind workflow_call -j soundness --input format_check_enabled=true
```

If you'd like `act` to always run with certain flags, these can be be placed in
an `.actrc` file either in the current working directory or your home
directory, for example:

```
--container-architecture=linux/amd64
--remote-name upstream
--action-offline-mode
```

For frequent contributors, we recommend adding the script as a [git pre-push hook](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks), which you can do via executing the following command in the project root directory:

Expand Down
3 changes: 0 additions & 3 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ let package = Package(
// code.
.package(url: "https://github.com/apple/swift-openapi-runtime", from: "1.3.2"),
.package(url: "https://github.com/apple/swift-http-types", from: "1.0.2"),

// Build and preview docs
.package(url: "https://github.com/apple/swift-docc-plugin", from: "1.3.0"),
],
targets: [

Expand Down
12 changes: 6 additions & 6 deletions docker/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,25 @@ services:

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you intend to keep the docker files around?

Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't actually considered removing them altogether but I guess the only downside to doing so is that the existing CI will immediately fail because it can't find them. Maybe that's fine since we want it turned off anyway.

@czechboy0 any objections to me just deleting all the docker-based CI files?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, let's get us moved over fully.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, I've realised that if we do this we'll be full of red X's on all PRs until we get them turned off because they'll still fire on the webhooks. Let's stage this in, get them turned off, then remove the files.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can turn off the web hooks yourself in the repo settings.

soundness:
<<: *common
command: /bin/bash -xcl "swift -version && uname -a && ./scripts/soundness.sh"
command: echo "skipping; moved to Github Actions"

test:
<<: *common
command: /bin/bash -xcl "swift $${SWIFT_TEST_VERB-test} $${WARN_AS_ERROR_ARG-} $${SANITIZER_ARG-} $${IMPORT_CHECK_ARG-} $${STRICT_CONCURRENCY_ARG-}"
command: echo "skipping; moved to Github Actions"

shell:
<<: *common
entrypoint: /bin/bash

integration-test:
<<: *common
command: /bin/bash -xcl "swift -version && uname -a && bash ./scripts/run-integration-test.sh"
command: echo "skipping; moved to Github Actions"
environment:
SWIFT_OPENAPI_GENERATOR_REPO_URL: file:///code

compatibility-test:
<<: *common
command: /bin/bash -xcl "cat /proc/cpuinfo && cat /proc/meminfo && swift test --filter $${SWIFT_OPENAPI_COMPATIBILITY_TEST_FILTER}"
command: echo "skipping; moved to Github Actions"
environment: # These can be overridden when running locally.
SWIFT_OPENAPI_COMPATIBILITY_TEST_ENABLE: "true"
SWIFT_OPENAPI_COMPATIBILITY_TEST_SKIP_BUILD: "true"
Expand All @@ -54,10 +54,10 @@ services:

docc-test:
<<: *common
command: /bin/bash -xcl "swift -version && uname -a && bash ./scripts/check-for-docc-warnings.sh"
command: echo "skipping; moved to Github Actions"
environment:
DOCC_TARGET: swift-openapi-generator

examples:
<<: *common
command: /bin/bash -xcl "swift -version && uname -a && bash ./scripts/test-examples.sh"
command: echo "skipping; moved to Github Actions"
37 changes: 0 additions & 37 deletions scripts/check-for-broken-symlinks.sh

This file was deleted.

40 changes: 0 additions & 40 deletions scripts/check-for-docc-warnings.sh

This file was deleted.

37 changes: 0 additions & 37 deletions scripts/check-for-unacceptable-language.sh

This file was deleted.

Loading