Skip to content

[#2704] Fixed SDC lint check passing when 'sdc-devel:validate' reports problems.#2705

Merged
AlexSkrypnyk merged 2 commits into
mainfrom
feature/2704-sdc-lint-fail
Jun 23, 2026
Merged

[#2704] Fixed SDC lint check passing when 'sdc-devel:validate' reports problems.#2705
AlexSkrypnyk merged 2 commits into
mainfrom
feature/2704-sdc-lint-fail

Conversation

@AlexSkrypnyk

@AlexSkrypnyk AlexSkrypnyk commented Jun 23, 2026

Copy link
Copy Markdown
Member

Closes #2704

Summary

The SDC lint gate introduced in #2671 was never actually enforced: drush sdc-devel:validate always exits 0 regardless of whether it found problems, so the check silently passed even when components had Twig errors or missing templates. This PR makes the gate functional by capturing the command output and pattern-matching against the severity labels that sdc_devel actually emits.

Changes

  • .ahoy.yml - lint-sdc command now captures drush sdc-devel:validate output and exits 1 when the output contains Critical, Error(s), or Warning(s). The exit-code check is replaced with an output grep because (a) the command always exits 0, and (b) the success message itself contains the word "errors" (lowercase), making a case-insensitive match impossible.
  • .github/workflows/build-test-deploy.yml - "Validate Single Directory Components" step applies the same capture-and-grep approach. The continue-on-error opt-out via VORTEX_CI_SDC_DEVEL_IGNORE_FAILURE is preserved.
  • .circleci/config.yml - Equivalent fix; the VORTEX_CI_SDC_DEVEL_IGNORE_FAILURE guard is preserved.
  • .vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php - Added subtestAhoyLintSdc with two assertions: ahoy lint-sdc passes on the valid shipped theme components (false-positive guard), and fails when a component template contains an unterminated Twig tag (proving the gate catches real breakage).
  • .vortex/tests/phpunit/Functional/AhoyWorkflowTest.php - Wired in subtestAhoyLintSdc.
  • Snapshot fixtures - Regenerated across all installer fixture variants to reflect the updated lint-sdc command body and CI step contents.

Before / After

BEFORE
------
ahoy lint-sdc
  └─ drush sdc-devel:validate
       exits 0 (always, even with problems)
         └─ step trusts exit code
              └─ always green (gate is toothless)

VORTEX_CI_SDC_DEVEL_IGNORE_FAILURE opt-out: inert (step never fails)


AFTER
-----
ahoy lint-sdc
  └─ drush sdc-devel:validate
       output captured (exit code ignored)
         └─ grep -qE 'Critical|Errors?|Warnings?'
              ├─ match found  -> exit 1  (build fails)
              └─ no match     -> exit 0  (build passes)

VORTEX_CI_SDC_DEVEL_IGNORE_FAILURE opt-out: functional (skips the exit 1)

Summary by CodeRabbit

  • Chores

    • Improved Single Directory Components (SDC) validation by capturing the linter’s combined output and failing when it contains critical/error/warning indicators, instead of relying on exit codes.
    • Applied the same output-scanning behavior consistently across linting in CI, while preserving the existing bypass option via an ignore-failure flag.
  • Tests

    • Expanded stateless workflow coverage to run an additional SDC lint subtest.
    • Added a test that intentionally breaks an SDC/Twig component, verifies the lint fails, then restores it and confirms lint passes again.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e2f3eead-d56c-48e5-ba50-797d9e3b9df7

📥 Commits

Reviewing files that changed from the base of the PR and between 40d8292 and 4bee8e6.

⛔ Files ignored due to path filters (61)
  • .vortex/installer/tests/Fixtures/handler_process/_baseline/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/_baseline/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/ciprovider_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/deploy_types_all_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/deploy_types_none_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/deploy_types_none_gha/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/deps_updates_provider_ci_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/migration_disabled_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/modules_no_sdc_devel/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/modules_no_sdc_devel/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/modules_none/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/modules_none/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/provision_profile/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_claro/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_claro/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_olivero/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_olivero/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_stark/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_stark/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/timezone_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_lint/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_lint_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_lint_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_behat/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_behat/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_behat_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_behat_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_no_theme/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_no_theme/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_jest/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_jest_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_jest_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpcs/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpcs_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpcs_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpstan_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_rector/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_rector_circleci/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_rector_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_circleci/.circleci/config.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_no_theme/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_no_theme/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_none/.ahoy.yml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_none/.github/workflows/build-test-deploy.yml is excluded by !.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (6)
  • .ahoy.yml
  • .circleci/config.yml
  • .circleci/vortex-test-common.yml
  • .github/workflows/build-test-deploy.yml
  • .vortex/tests/phpunit/Functional/AhoyWorkflowTest.php
  • .vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php

Walkthrough

Across .ahoy.yml, .circleci/config.yml, .github/workflows/build-test-deploy.yml, and .circleci/vortex-test-common.yml, the sdc-devel:validate invocation is changed from exit-code-based to output-pattern-based failure detection using case-sensitive grep for Critical, Error(s), or Warning(s). A new subtestAhoyLintSdc functional test covers both passing and failing lint scenarios.

Changes

SDC Lint Output-Based Failure Detection

Layer / File(s) Summary
Output-pattern failure detection in lint scripts
.ahoy.yml, .circleci/config.yml, .github/workflows/build-test-deploy.yml, .circleci/vortex-test-common.yml
Each lint environment now captures drush sdc-devel:validate output, echoes it, and exits 1 only when case-sensitive grep finds Critical, Error(s), or Warning(s) — avoiding false positives from the success message "No components errors found".
Functional subtest for ahoy lint-sdc
.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php, .vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php
subtestAhoyLintSdc is added and called from testAhoyWorkflowStateless(); it asserts ahoy lint-sdc passes for a valid component and fails after an unterminated Twig tag is injected, then restores the file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • drevops/vortex#2671: Introduced the initial SDC validation wiring (ahoy lint-sdc and CI "Validate Single Directory Components" steps) that this PR now corrects for exit-code behavior.

Poem

🐇 Oh the drush tool exits zero, it always does so,
But "errors" hides inside the success prose!
So now we grep with case, we parse the text,
Critical|Error|Warning — what comes next?
A clean build hops free, no false alarms in sight! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically summarizes the main change: fixing the SDC lint check that was incorrectly passing when validation problems were reported.
Linked Issues check ✅ Passed All changes fully align with issue #2704 requirements: case-sensitive pattern matching for Errors?/Warnings?/Critical in .ahoy.yml, CI workflows, and tests; VORTEX_CI_SDC_DEVEL_IGNORE_FAILURE opt-out preserved; test substep validates pass/fail behavior correctly.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the SDC lint check issue (#2704); no unrelated modifications or improvements are present in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/2704-sdc-lint-fail

Comment @coderabbitai help to get the list of available commands.

@github-actions

This comment has been minimized.

@AlexSkrypnyk

This comment has been minimized.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

📖 Documentation preview for this pull request has been deployed to Netlify:

https://6a3a88b68fb9e5d3bc4c0d10--vortex-docs.netlify.app

This preview is rebuilt on every commit and is not the production documentation site.

@AlexSkrypnyk

This comment has been minimized.

1 similar comment
@AlexSkrypnyk

This comment has been minimized.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.22%. Comparing base (12ce7b8) to head (4bee8e6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2705      +/-   ##
==========================================
- Coverage   86.67%   86.22%   -0.45%     
==========================================
  Files          96       89       -7     
  Lines        4719     4560     -159     
  Branches       47        3      -44     
==========================================
- Hits         4090     3932     -158     
+ Misses        629      628       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

This comment has been minimized.

@AlexSkrypnyk

This comment has been minimized.

2 similar comments
@AlexSkrypnyk

This comment has been minimized.

@AlexSkrypnyk

This comment has been minimized.

@AlexSkrypnyk AlexSkrypnyk added the Needs review Pull request needs a review from assigned developers label Jun 23, 2026
…s problems.

'sdc-devel:validate' always exits 0, so the ahoy and CI checks never failed on real component problems, and its success message contains the word 'errors', so a case-insensitive grep would also fail clean builds. Capture the command output and fail on a case-sensitive 'Critical'/'Error'/'Warning' match (advisory 'Notice' still passes), preserving the 'VORTEX_CI_SDC_DEVEL_IGNORE_FAILURE' opt-out. Added a functional test asserting the check passes on valid components and fails on a component with a Twig error.
@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/2704-sdc-lint-fail branch from 40d8292 to 4bee8e6 Compare June 23, 2026 13:10
@github-actions

Copy link
Copy Markdown

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   98.58% (208/211)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@AlexSkrypnyk

This comment has been minimized.

2 similar comments
@AlexSkrypnyk

This comment has been minimized.

@AlexSkrypnyk

Copy link
Copy Markdown
Member Author

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   98.58% (208/211)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@AlexSkrypnyk AlexSkrypnyk merged commit 6f78dca into main Jun 23, 2026
34 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/2704-sdc-lint-fail branch June 23, 2026 13:45
@github-project-automation github-project-automation Bot moved this from BACKLOG to Release queue in Vortex 1.x Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4 Board worker 4 Needs review Pull request needs a review from assigned developers

Projects

Status: Release queue

Development

Successfully merging this pull request may close these issues.

SDC Devel lint check: sdc-devel:validate exits 0 and its success message includes the word errors

1 participant