ci: Add Downstream Protobuf-Java Source Compatibility Test #3405
Conversation
lqiu96
force-pushed
the
downstream-protobuf-source-test
branch
from
ec2dcf9 to
bb30be6
Compare
lqiu96
force-pushed
the
downstream-protobuf-source-test
branch
from
e6911c5 to
642930d
Compare
| - 'release-please--branches--main' | ||
| workflow_dispatch: | ||
| inputs: | ||
| protobuf_versions: |
There was a problem hiding this comment.
Do we plan to use this for protoc testing as well? If not or not decided yet, maybe change this to something like protobuf_runtime_versions or protobuf_java_versions to make it clearer?
There was a problem hiding this comment.
(As of now) I don't plan to re-use this job for Protoc testing. It'll most likely be a separate job that is manually invoked to test for future Protoc RC and future versions.
I'll rename this param to reference the protobuf runtime versions
| pull_request: | ||
| # Updates on Release-Please PRs | ||
| branches: | ||
| - 'release-please--branches--main' |
There was a problem hiding this comment.
I thought this means this workflow would run on any PRs targeting 'release-please--branches--main', not PRs created from 'release-please--branches--main', can you verify it?
There was a problem hiding this comment.
Oh, I see. Yeah, I think you're right. It's for the branch that it's targeting and not where it is raised from.
I think there are other (not built in) solutions for this.
There was a problem hiding this comment.
Added a conditional that checks the head_ref (source branch) and the github.event type (schedule) so that it works for release please PRs and the nightly flow
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| repo: |
There was a problem hiding this comment.
Are all of them dependent on protobuf-java? For example, I don't see java-storage-nio is dependent on protobuf-java. Or we are adding all handwritten libraries here in case they start depending on protobuf-java in the future?
There was a problem hiding this comment.
I took a quick look and don't think they're all dependent on protobuf-java. java-storage-nio seems to be the odd one out that doesn't have any protobuf-java references at all and a few others have some references only in tests.
adding all handwritten libraries here in case they start depending on protobuf-java in the future
I think it'll be safer to keep all of the downstream libraries in here. Just to be safe in case there are references of Protobuf-Java added in source for downstream libraries.
|
|
||
| # Comma-delimited list of repos to test with the local java-shared-dependencies | ||
| if [ -z "${REPOS_UNDER_TEST}" ]; then | ||
| echo "REPOS_UNDER_TEST must be set to run downstream-protobuf-source-compatibility.sh" |
There was a problem hiding this comment.
Can we provide more detailed error hint if they are not supplied? For example, what are the expected format of the input etc. Same comment for PROTOBUF_RUNTIME_VERSION below.
There was a problem hiding this comment.
Sure, I can add an example
| cd "${scriptDir}/../.." # cd to the root of this repo | ||
| # common.sh lives in the presubmit folder | ||
| source "$scriptDir/../presubmit/common.sh" | ||
| setup_maven_mirror |
There was a problem hiding this comment.
Is this still needed now that we are overriding the property, not building sdk-platform-java anymore?
There was a problem hiding this comment.
I don't think we need to call setup_maven_mirror, but I kept it here for consistency with the other jobs that need to pull deps from Maven Central
I think setup_maven_mirror can be moved to a common.sh file that doesn't solely exist in the presubmit folder, but I didn't want to make that part of this PR.
There was a problem hiding this comment.
What is the difference if we don't call setup_maven_mirror?
There was a problem hiding this comment.
I don't recall the original context of using the maven mirror, but I believe it was to help speed up the CI jobs by reducing the time it takes to download the JARs. I just did a search and it doesn't look like it's being used anymore (maybe this was removed recently?).
I'll remove it since it doesn't seem to be used anymore.
|
|
Set up a nightly CI job that tests Protobuf compatibility with downstream libraries. This step of the CI job is to test source compatibility.
The nightly job will check the compatibility of these libraries with the latest in Protobuf-Java 3.x and and 4.x. This job can be manually invoked with the Protobuf-Java versions as a list.
Future steps may introduce testing for binary compatibility.