Skip to content

Run PR validation on multiple OS #312

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
+34 −8
Closed

Run PR validation on multiple OS #312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed40f3b
Run PR validation on multiple OS
skarpovdev Oct 10, 2025
0666928
Merge branch 'main' into skarpov/workflow
skarpovdev Oct 10, 2025
41b65a1
fixup! Run PR validation on multiple OS
skarpovdev Oct 10, 2025
9f8a26a
Merge branch 'main' into skarpov/workflow
skarpovdev Oct 11, 2025
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
37 changes: 29 additions & 8 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,26 @@ concurrency:
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}

jobs:
validate-pr:
runs-on: macos-latest-xlarge
name: Validate PR
build:
runs-on: ${{ matrix.os }}
name: ${{ matrix.name }}
strategy:
fail-fast: false
matrix:
include:
- os: macos-latest-xlarge
name: "macOS"
java-opts: "-Xmx8g -Dfile.encoding=UTF-8 -Djava.awt.headless=true -Dkotlin.daemon.jvm.options=-Xmx6g -DexcludeDockerTests=true"
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

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

The excludeDockerTests system property is set here but referenced as excludeDockerTests in the Gradle file, while the Gradle code checks for System.getProperty("excludeDockerTests"). Consider using a more descriptive property name like excludeDockerDependentTests to make the purpose clearer and ensure consistency between the workflow and build script.

Suggested change
java-opts: "-Xmx8g -Dfile.encoding=UTF-8 -Djava.awt.headless=true -Dkotlin.daemon.jvm.options=-Xmx6g -DexcludeDockerTests=true"
java-opts: "-Xmx8g -Dfile.encoding=UTF-8 -Djava.awt.headless=true -Dkotlin.daemon.jvm.options=-Xmx6g -DexcludeDockerDependentTests=true"

Copilot uses AI. Check for mistakes.

gradle-task: "clean ktlintCheck assemble macosArm64Test macosX64Test jvmTest koverLog koverHtmlReport"
test-type: "macos"
- os: ubuntu-latest
name: "Ubuntu"
java-opts: "-Xmx4g -Dfile.encoding=UTF-8 -Djava.awt.headless=true -Dkotlin.daemon.jvm.options=-Xmx3g"
gradle-task: "clean :kotlin-sdk-test:jvmTest"
test-type: "ubuntu"
env:
JAVA_OPTS: "-Xmx8g -Dfile.encoding=UTF-8 -Djava.awt.headless=true -Dkotlin.daemon.jvm.options=-Xmx6g"
JAVA_OPTS: "${{ matrix.java-opts }}"
TS_SDK_IMAGE: "registry.jetbrains.team/p/grazi/grazie-infra-public/typescript-sdk-mcp@sha256:69f7762ec271b768b10e2d383e1dbc135c4df38a314a75c1ef35e9ff42276cb4"
steps:
- uses: actions/checkout@v5

Expand All @@ -34,27 +49,32 @@ jobs:
cache-read-only: true

- name: Build with Gradle
run: |-
./gradlew clean ktlintCheck build koverLog koverHtmlReport
./gradlew :kotlin-sdk-core:publishToMavenLocal :kotlin-sdk-client:publishToMavenLocal :kotlin-sdk-server:publishToMavenLocal
run: ./gradlew ${{ matrix.gradle-task }}

- name: Publish to Maven Local (macOS only)
if: matrix.test-type == 'macos'
run: ./gradlew :kotlin-sdk-core:publishToMavenLocal :kotlin-sdk-client:publishToMavenLocal :kotlin-sdk-server:publishToMavenLocal

- name: Build Kotlin-MCP-Client Sample
if: matrix.test-type == 'macos'
working-directory: ./samples/kotlin-mcp-client
run: ./../../gradlew clean build

- name: Build Kotlin-MCP-Server Sample
if: matrix.test-type == 'macos'
working-directory: ./samples/kotlin-mcp-server
run: ./../../gradlew clean build

- name: Build Weather-Stdio-Server Sample
if: matrix.test-type == 'macos'
working-directory: ./samples/weather-stdio-server
run: ./../../gradlew clean build

- name: Upload Reports
if: ${{ !cancelled() }}
uses: actions/upload-artifact@v4
with:
name: reports
name: reports-${{ matrix.test-type }}
path: |
**/build/reports/

Expand All @@ -68,6 +88,7 @@ jobs:
include_empty_in_summary: false
include_time_in_summary: true
annotate_only: true
check_name: Test Report (${{ matrix.test-type }})

- name: Disable Auto-Merge on Fail
if: failure() && github.event_name == 'pull_request'
Expand Down
5 changes: 5 additions & 0 deletions kotlin-sdk-test/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ kotlin {
jvm {
testRuns["test"].executionTask.configure {
useJUnitPlatform()
if (System.getProperty("excludeDockerTests") == "true") {
filter {
excludeTestsMatching("*.typescript.*")
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

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

The test exclusion pattern *.typescript.* uses a generic wildcard that may inadvertently exclude tests beyond Docker-dependent ones. Consider using a more specific pattern like *DockerTest* or *IntegrationTest* to clearly identify which tests require Docker, making the exclusion logic more maintainable and less prone to accidentally excluding unrelated tests.

Suggested change
excludeTestsMatching("*.typescript.*")
excludeTestsMatching("*DockerTest*")

Copilot uses AI. Check for mistakes.

}
}
}
}
sourceSets {
Expand Down