Skip to content

Fix Pro dummy integration test gating #3697

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

Merged
justin808 merged 1 commit into from
Jun 6, 2026
Merged

Fix Pro dummy integration test gating #3697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 8 additions & 3 deletions .github/workflows/pro-integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ jobs:
non_runtime_only: ${{ steps.detect.outputs.non_runtime_only }}
run_pro_lint: ${{ steps.detect.outputs.run_pro_lint }}
run_pro_tests: ${{ steps.detect.outputs.run_pro_tests }}
run_pro_dummy_tests: ${{ steps.detect.outputs.run_pro_dummy_tests }}
has_full_ci_label: ${{ steps.check-label.outputs.result }}
steps:
- uses: actions/checkout@v4
Expand All @@ -66,6 +67,7 @@ jobs:
{
echo "run_pro_lint=true"
echo "run_pro_tests=true"
echo "run_pro_dummy_tests=true"
echo "docs_only=false"
echo "non_runtime_only=false"
} >> "$GITHUB_OUTPUT"
Expand Down Expand Up @@ -93,7 +95,8 @@ jobs:
) && (
github.ref == 'refs/heads/main' ||
github.event_name == 'workflow_dispatch' ||
needs.detect-changes.outputs.run_pro_tests == 'true'
needs.detect-changes.outputs.run_pro_tests == 'true' ||
needs.detect-changes.outputs.run_pro_dummy_tests == 'true'
)
runs-on: ubuntu-22.04
env:
Expand Down Expand Up @@ -193,7 +196,8 @@ jobs:
) && (
github.ref == 'refs/heads/main' ||
github.event_name == 'workflow_dispatch' ||
needs.detect-changes.outputs.run_pro_tests == 'true'
needs.detect-changes.outputs.run_pro_tests == 'true' ||
needs.detect-changes.outputs.run_pro_dummy_tests == 'true'
)
runs-on: ubuntu-22.04
env:
Expand Down Expand Up @@ -369,7 +373,8 @@ jobs:
) && (
github.ref == 'refs/heads/main' ||
github.event_name == 'workflow_dispatch' ||
needs.detect-changes.outputs.run_pro_tests == 'true'
needs.detect-changes.outputs.run_pro_tests == 'true' ||
needs.detect-changes.outputs.run_pro_dummy_tests == 'true'
)
runs-on: ubuntu-22.04
env:
Expand Down
20 changes: 20 additions & 0 deletions script/ci-changes-detector-test.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ setup_repo() {
mkdir -p react_on_rails/spec/react_on_rails
mkdir -p react_on_rails/app/helpers
mkdir -p react_on_rails_pro/app/controllers/react_on_rails_pro/rolling_deploy
mkdir -p react_on_rails_pro/spec/dummy/spec/requests
mkdir -p benchmarks/lib
cat > docs/guide.md <<'DOC'
# Guide
Expand Down Expand Up @@ -157,6 +158,13 @@ module ReactOnRailsPro
end
end
end
RUBY
cat > react_on_rails_pro/spec/dummy/spec/requests/posts_page_spec.rb <<'RUBY'
RSpec.describe "posts page" do
it "renders" do
expect(true).to be(true)
end
end
RUBY

git add .
Expand Down Expand Up @@ -454,6 +462,17 @@ test_pro_app_source_change_runs_pro_tests_only() {
assert_contains "$out" '"run_js_tests": false' "pro app source output"
}

test_pro_dummy_only_change_runs_pro_dummy_tests_without_pro_unit_tests() {
setup_repo
perl -0pi -e 's/renders/renders seeded data/' react_on_rails_pro/spec/dummy/spec/requests/posts_page_spec.rb
commit_change "pro dummy spec"

local out
out="$(detector_output)"
assert_contains "$out" '"run_pro_dummy_tests": true' "pro dummy output"
assert_contains "$out" '"run_pro_tests": false' "pro dummy output"
}
Comment on lines +465 to +474

@greptile-apps greptile-apps Bot Jun 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Missing scope-narrowing assertions

The test confirms the two key flags but doesn't assert that a Pro-dummy-only change doesn't bleed into the core gem suite. The analogous test test_pro_app_source_change_runs_pro_tests_only explicitly checks run_ruby_tests: false and run_js_tests: false with the comment "Scoped to Pro: a Pro app change must not drag in the core gem suite." Adding the same guards here would catch a future regression where the Pro-dummy path accidentally triggers core-gem jobs.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


test_pro_node_renderer_comment_only_change_runs_pro_lint_only() {
setup_repo
perl -0pi -e 's/export function/\/\/ Explains the Pro node renderer fixture.\nexport function/' \
Expand Down Expand Up @@ -674,6 +693,7 @@ run_test test_core_app_comment_only_change_skips_heavy_tests_but_keeps_lint
run_test test_core_app_source_change_remains_runtime_affecting
run_test test_pro_app_comment_only_change_runs_pro_lint_only
run_test test_pro_app_source_change_runs_pro_tests_only
run_test test_pro_dummy_only_change_runs_pro_dummy_tests_without_pro_unit_tests
run_test test_pro_node_renderer_comment_only_change_runs_pro_lint_only
run_test test_mixed_comment_and_code_change_remains_runtime_affecting
run_test test_ruby_magic_comment_remains_runtime_affecting
Expand Down
Loading