diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 6511d1e8..1ac7d02a 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -41,7 +41,15 @@ semantic_pull_request: - Examples: `feat: add new feature`, `fix(mesh): resolve bug` ``` -#### 2. Build and Test +#### 2. Draft PR Check +```yaml +check_draft: + - Fails CI if PR is still in draft mode + - Prevents accidental merging of incomplete work + - Mark PR as "Ready for review" to proceed +``` + +#### 3. Build and Test ```yaml build: - Matrix testing across Python 3.10, 3.11, 3.12 @@ -50,7 +58,16 @@ build: - Runs pytest tests ``` -#### 3. GEOS Integration Check +#### 4. Label Checks +```yaml +check_integration_label: + - Checks for 'test-geos-integration' label + +check_force_integration_label: + - Checks for 'force-geos-integration' label +``` + +#### 5. GEOS Integration Required Check ```yaml check_geos_integration_required: - Analyzes changed files @@ -58,14 +75,22 @@ check_geos_integration_required: - See "Smart GEOS Integration Testing" section below ``` -#### 4. GEOS Integration Test (Conditional) +#### 6. GEOS CI Dispatch +```yaml +geos_ci_dispatch: + - Evaluates label + file change requirements + - Decides whether to run, skip, or fail + - Provides clear error messages for incorrect label usage +``` + +#### 7. GEOS Integration Test (Conditional) ```yaml geos_integration_test: - - Only runs if required by file changes or label + - Only runs if dispatch job says to proceed - Calls test_geos_integration.yml workflow ``` -#### 5. Final Validation +#### 8. Final Validation ```yaml final_validation: - Summarizes all test results @@ -249,35 +274,78 @@ Tests are automatically skipped when changes only affect: - `.github/workflows/doc-test.yml` - Documentation CI - `.github/workflows/typing-check.yml` - Type checking CI -### Manual Override +### Manual Override Labels + +The CI supports two labels for controlling GEOS integration tests: + +1. **`test-geos-integration`** - Must be added when changes affect GEOS-integrated packages + - Required when modifying: `geos-utils`, `geos-mesh`, `geos-xml-tools`, `hdf5-wrapper`, `pygeos-tools`, `geos-ats` + - CI will fail if this label is missing when required + - CI will warn if this label is present but not needed (suggest using `force-geos-integration` instead) + +2. **`force-geos-integration`** - Forces tests to run regardless of changed files + - Use this when testing CI changes themselves + - Use this for docs/config-only PRs where you want to verify GEOS integration still works + - Overrides all other logic - tests will always run + +### Label Decision Logic + +The CI uses the following decision matrix: -Add the **`test-geos-integration`** label to any PR to force GEOS integration tests to run, regardless of changed files. +| GEOS Required
(by file changes) | `test-geos-integration`
label | `force-geos-integration`
label | Result | +|-------------------------------------|-----------------------------------|-------------------------------------|--------| +| - | - | ✅ Yes | ✅ **Run tests** (forced) | +| ✅ Yes | ✅ Yes | ❌ No | ✅ **Run tests** (normal) | +| ✅ Yes | ❌ No | ❌ No | ❌ **ERROR** - Label required | +| ❌ No | ✅ Yes | ❌ No | ⊘ **Skip** - Wrong label (use force instead) | +| ❌ No | ❌ No | ❌ No | ⊘ **Skip tests** (not needed) | ### Example Scenarios -✅ **Tests Will Run** +✅ **Tests Will Run (Required + Label)** ``` Changes: - geos-mesh/src/mesh_converter.py - geos-xml-tools/src/preprocessor.py -Result: GEOS integration required (affects integrated packages) +Labels: test-geos-integration +Result: GEOS integration will run (changes affect integrated packages) ``` -⊘ **Tests Will Skip** +❌ **CI Will Fail (Required but Missing Label)** +``` +Changes: + - geos-utils/src/table_loader.py + - hdf5-wrapper/src/wrapper.py +Labels: (none) +Result: ERROR - 'test-geos-integration' label required +``` + +⊘ **Tests Will Skip (Not Required)** ``` Changes: - docs/user_guide.md - README.md - geos-pv/src/visualizer.py -Result: GEOS integration not required (only docs and non-integrated packages) +Labels: (none) +Result: GEOS integration not required (skipped) ``` -✅ **Tests Will Run (Manual)** +⊘ **Tests Will Skip (Wrong Label)** ``` Changes: - docs/installation.md + - .style.yapf Labels: test-geos-integration -Result: GEOS integration required (manual override via label) +Result: Warning - Label not needed, use 'force-geos-integration' to force tests (skipped) +``` + +✅ **Tests Will Run (Forced)** +``` +Changes: + - docs/installation.md + - .github/workflows/python-package.yml +Labels: force-geos-integration +Result: GEOS integration forced (tests will run regardless of changes) ``` ## GEOS Integration: How It Works @@ -393,6 +461,23 @@ When adding new Python packages or modifying existing ones: 4. **Test Integration**: - If package integrates with GEOS, add `test-geos-integration` label to PR - Verify all 5 integration tests pass + - If only testing CI changes on a docs PR, use `force-geos-integration` label + +### PR Label Usage Guide + +**For PR Authors:** + +| Your PR Changes | Required Label | What Happens | +|-----------------|----------------|--------------| +| GEOS-integrated packages
(utils, mesh, xml-tools, etc.) | `test-geos-integration` | ✅ Tests run (required) | +| Docs/config only | (none) | ⊘ Tests skipped | +| Docs + you want to test integration | `force-geos-integration` | ✅ Tests run (forced) | +| CI workflow files | `force-geos-integration` | ✅ Tests run (verify CI works) | + +**Common Mistakes:** +- ❌ Modifying `geos-mesh/` without label → CI fails, add `test-geos-integration` +- ❌ Adding `test-geos-integration` to docs-only PR → Warning, remove label or use `force-geos-integration` +- ✅ Modifying `.github/workflows/` with `force-geos-integration` → Tests run to verify CI changes ## References diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 94c49978..a4c6823c 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -33,14 +33,29 @@ jobs: run: | echo "This is not a Pull-Request, skipping" + check_draft: + runs-on: ubuntu-latest + needs: [semantic_pull_request] + steps: + - name: Fail if PR is a draft + run: | + if [ "${{ github.event.pull_request.draft }}" = "true" ]; then + echo "✗ This PR is a draft. CI will not proceed." + exit 1 + else + echo "✓ This PR is ready for review." + fi + + # build and test the standalone CI build: runs-on: ubuntu-latest + needs: [check_draft] strategy: fail-fast: false max-parallel: 3 matrix: - python-version: ["3.10", "3.11", "3.12"] - package-name: + python-version: ["3.10","3.11","3.12"] + package-name: - geos-ats - geos-utils - geos-geomechanics @@ -88,7 +103,6 @@ jobs: echo "Installing main package..." python -m pip install ./${{ matrix.package-name }}/[test] - - name: Lint with yapf # working-directory: ./${{ matrix.package-name }} run: | @@ -100,11 +114,52 @@ jobs: # wrap pytest to avoid error when no tests in the package sh -c 'python -m pytest ./${{ matrix.package-name }}; ret=$?; [ $ret = 5 ] && exit 0 || exit $ret' + # check if GEOS has label for testing GEOS integration + check_integration_label: + runs-on: ubuntu-latest + needs: [build] + outputs: + has_geos_integration_label: ${{ steps.set-label.outputs.has_label }} + steps: + - name: Check if PR has 'test-geos-integration' label + id: set-label + run: | + echo "Checking for label..." + LABEL_FOUND=false + for label in "${{ toJson(github.event.pull_request.labels) }}"; do + if [[ "$label" == *"test-geos-integration"* ]]; then + LABEL_FOUND=true + echo "Label ${label} found" + break + fi + done + echo "has_label=$LABEL_FOUND" >> $GITHUB_OUTPUT + + check_force_integration_label: + runs-on: ubuntu-latest + needs: [build] + outputs: + has_geos_integration_force_label: ${{ steps.set-label.outputs.has_label }} + steps: + - name: Check if PR has 'force-geos-integration' label + id: set-label + run: | + echo "Checking for label..." + LABEL_FOUND=false + for label in "${{ toJson(github.event.pull_request.labels) }}"; do + if [[ "$label" == *"force-geos-integration"* ]]; then + LABEL_FOUND=true + echo "Label ${label} found" + break + fi + done + echo "has_label=$LABEL_FOUND" >> $GITHUB_OUTPUT + # Step 3: Check if GEOS integration is required based on changed files check_geos_integration_required: name: Check GEOS Integration Required runs-on: ubuntu-latest - needs: [semantic_pull_request, build] + needs: [build] if: github.event_name == 'pull_request' outputs: geos_integration_required: ${{ steps.check_changes.outputs.required }} @@ -114,7 +169,6 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 0 # Fetch all history to compare with base branch - - name: Check if GEOS integration is required id: check_changes run: | @@ -159,16 +213,6 @@ jobs: "^geos-xml-viewer/" ) - # Check if label is present (overrides automatic detection) - HAS_LABEL=$(echo '${{ toJson(github.event.pull_request.labels.*.name) }}' | grep -q "test-geos-integration" && echo "true" || echo "false") - - if [[ "$HAS_LABEL" == "true" ]]; then - echo "✓ Label 'test-geos-integration' found - GEOS integration test will run" - echo "required=true" >> "$GITHUB_OUTPUT" - echo "skip_reason=none" >> "$GITHUB_OUTPUT" - exit 0 - fi - # Check if any changed file affects GEOS-integrated packages REQUIRES_GEOS_TEST=false AFFECTED_PACKAGES="" @@ -221,19 +265,99 @@ jobs: echo "required=false" >> "$GITHUB_OUTPUT" echo "skip_reason=no-geos-integrated-changes" >> "$GITHUB_OUTPUT" fi + + geos_ci_dispatch: + name: Dispatch cases of GEOS CI + runs-on: ubuntu-latest + outputs: + is_GEOS_CI_skipped: ${{ steps.dispatch.outputs.skipped }} + fwd_geos_integration_required: ${{ steps.dispatch.outputs.fwd_geos_integration_required }} + fwd_skip_reason: ${{ steps.dispatch.outputs.skip_reason }} + needs: [check_geos_integration_required, check_integration_label, check_force_integration_label] + steps: + - name: Dispatch flag and req cases + id: dispatch + run: | + GEOS_REQUIRED="${{ needs.check_geos_integration_required.outputs.geos_integration_required }}" + HAS_TEST_LABEL="${{ needs.check_integration_label.outputs.has_geos_integration_label }}" + HAS_FORCE_LABEL="${{ needs.check_force_integration_label.outputs.has_geos_integration_force_label }}" + SKIP_REASON="${{ needs.check_geos_integration_required.outputs.skip_reason }}" + + echo "fwd_geos_integration_required=${GEOS_REQUIRED}" >> "$GITHUB_OUTPUT" + echo "fwd_skip_reason=${SKIP_REASON}" >> "$GITHUB_OUTPUT" + + echo "=== GEOS Integration Dispatch ===" + echo "GEOS Required (by file changes): ${GEOS_REQUIRED}" + echo "Has 'test-geos-integration' label: ${HAS_TEST_LABEL}" + echo "Has 'force-geos-integration' label: ${HAS_FORCE_LABEL}" + echo "" + + # Case 1: Force label - always run tests + if [[ "$HAS_FORCE_LABEL" == "true" ]]; then + echo "✓ 'force-geos-integration' label present - forcing GEOS integration tests" + echo "skipped=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Case 2: GEOS required AND test label present - run tests + if [[ "$GEOS_REQUIRED" == "true" && "$HAS_TEST_LABEL" == "true" ]]; then + echo "✓ GEOS integration required and 'test-geos-integration' label present" + echo " Will proceed with GEOS integration tests" + echo "skipped=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Case 3: GEOS required BUT test label missing - ERROR + if [[ "$GEOS_REQUIRED" == "true" && "$HAS_TEST_LABEL" == "false" ]]; then + echo "✗ ERROR: GEOS integration is required but 'test-geos-integration' label is missing" + echo "" + echo "Your PR modifies GEOS-integrated packages:" + echo " - geos-utils, geos-mesh, geos-xml-tools" + echo " - hdf5-wrapper, pygeos-tools, geos-ats" + echo "" + echo "Action required: Add the 'test-geos-integration' label to this PR" + exit 1 + fi + + # Case 4: GEOS NOT required BUT test label present - SKIP TESTS + if [[ "$GEOS_REQUIRED" == "false" && "$HAS_TEST_LABEL" == "true" ]]; then + echo "⊘ SKIPPED: 'test-geos-integration' label present but GEOS integration is not required" + echo "" + echo "Your changes only affect:" + echo " - Documentation" + echo " - Non-integrated packages" + echo " - Configuration files" + echo "" + echo "If you want to run GEOS integration tests anyway, use 'force-geos-integration' label to explicitly force testing" + echo "skipped=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Case 5: GEOS NOT required AND no labels - SKIP TESTS + if [[ "$GEOS_REQUIRED" == "false" && "$HAS_TEST_LABEL" == "false" ]]; then + echo "⊘ GEOS integration not required and no labels present" + echo " Skipping GEOS integration tests" + echo " Reason: ${SKIP_REASON}" + echo "skipped=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Should never reach here + echo "✗ ERROR: Unexpected state in dispatch logic" + exit 1 # Step 4: Run GEOS integration tests (only if required or label present) geos_integration_test: name: GEOS Integration Test - needs: [check_geos_integration_required] - if: needs.check_geos_integration_required.outputs.geos_integration_required == 'true' + needs: [geos_ci_dispatch] + if: ${{ needs.geos_ci_dispatch.outputs.is_GEOS_CI_skipped.skipped != 'true' }} uses: ./.github/workflows/test_geos_integration.yml # Final validation - Summarize CI results final_validation: name: Final CI Validation runs-on: ubuntu-latest - needs: [check_geos_integration_required, geos_integration_test] + needs: [geos_ci_dispatch, geos_integration_test] if: always() && github.event_name == 'pull_request' steps: - name: Validate CI completion @@ -241,8 +365,9 @@ jobs: echo "Final CI Validation" echo "===================" - GEOS_REQUIRED="${{ needs.check_geos_integration_required.outputs.geos_integration_required }}" - SKIP_REASON="${{ needs.check_geos_integration_required.outputs.skip_reason }}" + GEOS_REQUIRED="${{ needs.geos_ci_dispatch.outputs.fwd_geos_integration_required }}" + SKIP_REASON="${{ needs.geos_ci_dispatch.outputs.fwd_skip_reason }}" + GEOS_RESULT="${{ needs.geos_integration_test.result }}" if [[ "$GEOS_REQUIRED" == "true" ]]; then diff --git a/.github/workflows/test_geos_integration.yml b/.github/workflows/test_geos_integration.yml index 2fb9773a..d4c40d34 100644 --- a/.github/workflows/test_geos_integration.yml +++ b/.github/workflows/test_geos_integration.yml @@ -282,14 +282,13 @@ jobs: fi integration_summary: - name: GEOS Integration Summary + name: GEOS Integration Test Summary runs-on: ubuntu-latest needs: test_geos_integration if: always() steps: - name: Summarize Results run: | - echo "## GEOS Integration Test Summary" >> $GITHUB_STEP_SUMMARY if [[ "${{ needs.test_geos_integration.result }}" == "success" ]]; then echo "All integration tests passed successfully!" >> $GITHUB_STEP_SUMMARY else