[GitHub] [lucene] dweiss commented on a diff in pull request #893: LUCENE-10531: Run GUI tests on CI only

[GitBox <gi...@apache.org>]

dweiss commented on code in PR #893:
URL: https://github.com/apache/lucene/pull/893#discussion_r874090898


##########
lucene/distribution.tests/build.gradle:
##########
@@ -50,10 +50,15 @@ test {
   jvmArgumentProviders.add(new CommandLineArgumentProvider() {
     @Override
     Iterable<String> asArguments() {
-      return [
+      var args = [
           "-Dlucene.distribution.dir=${configurations.binaryDistribution.singleFile.absolutePath }",
           "-Dlucene.distribution.version=${project.version}"
       ]
+      if (isCIBuild) {

Review Comment:
   I would move this to be a dynamically computed default value in randomization.gradle.



##########
.github/workflows/distribution.yml:
##########
@@ -0,0 +1,61 @@
+name: Distribution tests
+
+on:
+  pull_request:
+    branches:
+      - 'main'
+
+jobs:
+  test:
+    name: Run distribution tests
+
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        # we run the distribution tests on all major OSs.
+        os: [ubuntu-latest, macos-latest, windows-latest]
+
+    steps:
+    - uses: actions/checkout@v2
+
+    - name: Set up JDK
+      uses: actions/setup-java@v2
+      with:
+        distribution: 'temurin'
+        java-version: 17
+        java-package: jdk
+
+    - name: Save/Restore gradle cache (${{ matrix.os }})
+      uses: actions/cache@v2
+      if: ${{ startsWith(matrix.os, 'windows') }}
+      with:
+        path: |
+           %HOMEDRIVE%%HOMEPATH%\.gradle\caches
+        key: ${{ runner.os }}-gradle-distribution-${{ hashFiles('versions.lock') }}
+        restore-keys: |
+          ${{ runner.os }}-gradle-distribution-
+          ${{ runner.os }}-gradle-
+    - name: Save/Restore gradle cache (${{ matrix.os }})
+      uses: actions/cache@v2
+      if: ${{ ! startsWith(matrix.os, 'windows') }}
+      with:
+        path: |
+          ~/.gradle/caches
+        key: ${{ runner.os }}-gradle-distribution-${{ hashFiles('versions.lock') }}
+        restore-keys: |
+          ${{ runner.os }}-gradle-distribution-
+          ${{ runner.os }}-gradle-
+
+    - name: Initialize gradle settings (${{ matrix.os }})
+      if: ${{ startsWith(matrix.os, 'windows') }}
+      run: .\gradlew localSettings
+    - name: Initialize gradle settings (${{ matrix.os }})
+      if: ${{ ! startsWith(matrix.os, 'windows') }}
+      run: ./gradlew localSettings
+
+    - name: Run all distribution tests including GUI tests (${{ matrix.os }})
+      if: ${{ startsWith(matrix.os, 'windows') }}
+      run: .\gradlew -p "lucene\distribution.tests" test

Review Comment:
   I think windows runs with powershell and ./gradlew should work on both platforms, actually (no need for duplication). Carrot2 runs on both from a single workflow definition:
   
   https://github.com/carrot2/carrot2/actions/runs/2329856785/workflow
   https://github.com/carrot2/carrot2/actions/runs/2329856785
   
   The paths for gradle can contain slashes on Windows as well.



##########
.github/workflows/hunspell.yml:
##########
@@ -22,15 +22,13 @@ jobs:
         distribution: 'temurin'
         java-version: 17
         java-package: jdk
-    - name: Grant execute permission for gradlew
-      run: chmod +x gradlew
     - uses: actions/cache@v2
       with:
         path: |
           ~/.gradle/caches
-        key: ${{ runner.os }}-gradle-solrj-${{ hashFiles('versions.lock') }}

Review Comment:
   The key here should be identical so that cache is reused for the same os+versions.lock - the infix is not required, actually - it makes the cache miss files that would otherwise be reused across jobs.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org