Integration tests for the build plugin #31

rnro · 2025-01-21T15:50:40Z

Motivation:

To protect against regressions in common use-cases of the grpc-swift-protobuf build plugin.

Modifications:

Add test cases which make use of the build plugin as a dependency and ensure that they can compile and use the generated code

top level config file
peer config file
separate service message protos
cross directory imports
two definitions
nested definitions

The new tests are run as part of CI on PRs

Result:

More CI.

This PR was split out of #26

glbrntt

I have some feedback and you might not like it because it involves writing more bash... 😬

There's a lot of copy-pasta here which I think we can script quite easily (I think).

All the .gitignores are the same
All the Package.swift files are the same
The hello world proto is used in tests 1, 2, 5, and 6
The adopter.swift file is the same for tests 1, 2, 3, and 4, another adopter.swift file is the same for tests 5 and 6.

I think the approach here should be to generate out packages into a temporary directory and construct a package in there by blatting in a package manifest (we can skip the ignore file), creating a Sources directory and then populating it as necessary.

glbrntt · 2025-01-21T18:06:35Z

.github/workflows/matrices/plugin-tests.json

@@ -0,0 +1 @@
+{"config":[{"name":"Plugin tests (6.0)","swift_version":"6.0","runner":"ubuntu-latest","image":"swift:6.0-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh | bash","command_arguments":""},{"name":"Plugin tests (nightly-6.0)","swift_version":"nightly-6.0","runner":"ubuntu-latest","image":"swiftlang/swift:nightly-6.0-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh | bash","command_arguments":""},{"name":"Plugin tests (nightly-main)","swift_version":"nightly-main","runner":"ubuntu-latest","image":"swiftlang/swift:nightly-main-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh | bash","command_arguments":""}]}


Is this generated somehow, if so, how? Can we run it through jq or similar so it's readable.

It is not generated. We can run it through jq to make it readable for the purposes of review but the loading workflow requires that the JSON be densely encoded (jq . -c).

{ "config": [ { "name": "Plugin tests (6.0)", "swift_version": "6.0", "runner": "ubuntu-latest", "image": "swift:6.0-jammy", "platform": "Linux", "setup_command": "apt-get update -y -q && apt-get install -y -q curl protobuf-compiler", "command": "curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh | bash", "command_arguments": "" }, { "name": "Plugin tests (nightly-6.1)", "swift_version": "nightly-6.1", "runner": "ubuntu-latest", "image": "swiftlang/swift:nightly-6.1-jammy", "platform": "Linux", "setup_command": "apt-get update -y -q && apt-get install -y -q curl protobuf-compiler", "command": "curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh | bash", "command_arguments": "" }, { "name": "Plugin tests (nightly-main)", "swift_version": "nightly-main", "runner": "ubuntu-latest", "image": "swiftlang/swift:nightly-main-jammy", "platform": "Linux", "setup_command": "apt-get update -y -q && apt-get install -y -q curl protobuf-compiler", "command": "curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh | bash", "command_arguments": "" } ] }

Out of interest why does it need to be densely encoded?

The GitHub action which loads matrices from JSON errors-out if it encounters any unnecessary whitespace. It's pretty great. It used to have a step which pre-processed and condensed the JSON but it was noisy in CI so it was removed.

.github/workflows/main.yml

glbrntt · 2025-01-21T18:15:01Z

dev/plugin-tests.sh

+error() { printf -- "** ERROR: %s\n" "$*" >&2; }
+fatal() { error "$@"; exit 1; }
+
+source_dir=$(pwd)


This assumes the current working directory is the root of the package, which is quite brittle.

Most other shell scripts take the location of the shell script and then figure out where the root of the package is relative to that. which is less brittle. Here's an example:

grpc-swift-protobuf/dev/protos/generate.sh

Line 18 in 0e6d93b

here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

So the reason I didn't do this (which I forgot) was that we tend to invoke scripts by curling and piping them into bash. In that case we don't have a source directory to anchor our relative paths to. In that case our scripts just assume you're in the right place. I've added some code which attempts to handle both.

glbrntt · 2025-01-21T18:15:21Z

dev/plugin-tests.sh

+fatal() { error "$@"; exit 1; }
+
+source_dir=$(pwd)
+pluginTests="${source_dir}/IntegrationTests/PluginTests"


snake_case for shell scripts please!

glbrntt · 2025-01-21T18:15:37Z

dev/plugin-tests.sh

+
+for dir in "$pluginTests"/test_*/ ; do
+  if [[ -f "$dir/Package.swift" ]]; then
+    pluginTest=$(basename "$dir")


snake_case please!

glbrntt · 2025-01-21T18:16:38Z

IntegrationTests/PluginTests/test_01_top_level_config_file/Package.swift

+    ),
+    .package(
+      url: "https://github.com/grpc/grpc-swift.git",
+      exact: "2.0.0-beta.3"


We only set to exact: for when we tag releases so that's unlikely to be correct (grpc-swift-protobuf is likely to change its requirement between on grpc-swift very soon).

Motivation: To protect against regressions in common use-cases of the grpc-swift-protobuf build plugin. Modifications: Add test cases which make use of the build plugin as a dependency and ensure that they can compile and use the generated code * top level config file * peer config file * separate service message protos * cross directory imports * two definitions * nested definitions The new tests are run as part of CI on PRs Result: More CI.

glbrntt

Couple of small bits but looks good otherwise, thanks Rick!

glbrntt · 2025-01-23T13:52:53Z

.github/workflows/matrices/plugin-tests.json

@@ -0,0 +1 @@
+{"config":[{"name":"Plugin tests (6.0)","swift_version":"6.0","runner":"ubuntu-latest","image":"swift:6.0-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/rnro/grpc-swift-protobuf/refs/heads/build_plugin_integration_tests/dev/plugin-tests.sh| GITHUB_ACTIONS=true bash","command_arguments":""},{"name":"Plugin tests (nightly-6.1)","swift_version":"nightly-6.1","runner":"ubuntu-latest","image":"swiftlang/swift:nightly-6.1-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/rnro/grpc-swift-protobuf/refs/heads/build_plugin_integration_tests/dev/plugin-tests.sh| GITHUB_ACTIONS=true bash","command_arguments":""},{"name":"Plugin tests (nightly-main)","swift_version":"nightly-main","runner":"ubuntu-latest","image":"swiftlang/swift:nightly-main-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/rnro/grpc-swift-protobuf/refs/heads/build_plugin_integration_tests/dev/plugin-tests.sh| GITHUB_ACTIONS=true bash","command_arguments":""}]}


Late realisation: if we use this vs. this approach we have to update it for each new Swift version. Can we do the alternative?

glbrntt · 2025-01-23T13:53:42Z

IntegrationTests/PluginTests/Resources/Package.swift

+
+import PackageDescription
+
+var package = Package(


nit: this should be a let as Package is a class 🙃

rnro force-pushed the build_plugin_integration_tests branch 2 times, most recently from 25e4eeb to 359affd Compare January 21, 2025 17:20

rnro added the 🔨 semver/patch No public API change. label Jan 21, 2025

rnro force-pushed the build_plugin_integration_tests branch 2 times, most recently from 1368b0f to 3c34daa Compare January 21, 2025 17:23

rnro requested a review from glbrntt January 21, 2025 17:27

rnro marked this pull request as ready for review January 21, 2025 17:27

glbrntt requested changes Jan 21, 2025

View reviewed changes

rnro added 2 commits January 22, 2025 06:52

review comments

3c5b627

rnro force-pushed the build_plugin_integration_tests branch from 58c07c6 to 3c5b627 Compare January 22, 2025 06:54

rnro added 2 commits January 23, 2025 08:51

dynamically create plugin tests

e2d4cab

TEMPORARY URL

7105fb7

rnro force-pushed the build_plugin_integration_tests branch from 8f63edd to 7105fb7 Compare January 23, 2025 08:52

rnro added 2 commits January 23, 2025 08:58

remove custom temp dir prefix

658670f

formatting

d8dc444

rnro force-pushed the build_plugin_integration_tests branch 4 times, most recently from 376b6db to c6201ec Compare January 23, 2025 10:12

plugin-tests.sh handles being sourced

63dc0bf

rnro force-pushed the build_plugin_integration_tests branch from c6201ec to 63dc0bf Compare January 23, 2025 11:12

Merge branch 'main' into build_plugin_integration_tests

b8e1d21

rnro requested a review from glbrntt January 23, 2025 11:21

glbrntt requested changes Jan 23, 2025

View reviewed changes

CI generate plugin test matrix

6c3f38a

glbrntt approved these changes Jan 24, 2025

View reviewed changes

Merge branch 'main' into build_plugin_integration_tests

78620b0

glbrntt merged commit 4153ada into grpc:main Jan 24, 2025
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration tests for the build plugin #31

Integration tests for the build plugin #31

rnro commented Jan 21, 2025 •

edited

Loading

glbrntt left a comment

glbrntt Jan 21, 2025

rnro Jan 22, 2025

glbrntt Jan 22, 2025

rnro Jan 23, 2025 •

edited

Loading

glbrntt Jan 21, 2025

rnro Jan 23, 2025

glbrntt Jan 21, 2025

glbrntt Jan 21, 2025

glbrntt Jan 21, 2025

glbrntt left a comment

glbrntt Jan 23, 2025

glbrntt Jan 23, 2025

		@@ -0,0 +1 @@
		{"config":[{"name":"Plugin tests (6.0)","swift_version":"6.0","runner":"ubuntu-latest","image":"swift:6.0-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh \| bash","command_arguments":""},{"name":"Plugin tests (nightly-6.0)","swift_version":"nightly-6.0","runner":"ubuntu-latest","image":"swiftlang/swift:nightly-6.0-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh \| bash","command_arguments":""},{"name":"Plugin tests (nightly-main)","swift_version":"nightly-main","runner":"ubuntu-latest","image":"swiftlang/swift:nightly-main-jammy","platform":"Linux","setup_command":"apt-get update -y -q && apt-get install -y -q curl protobuf-compiler","command":"curl -s https://raw.githubusercontent.com/grpc/grpc-swift-protobuf/package_plugins/dev/plugin-tests.sh \| bash","command_arguments":""}]}

Integration tests for the build plugin #31

Integration tests for the build plugin #31

Conversation

rnro commented Jan 21, 2025 • edited Loading

Motivation:

Modifications:

Result:

glbrntt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnro Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glbrntt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnro commented Jan 21, 2025 •

edited

Loading

rnro Jan 23, 2025 •

edited

Loading