diff --git a/.github/actions/setup-benchmark-runner/action.yml b/.github/actions/setup-benchmark-runner/action.yml deleted file mode 100644 index 0232838fee..0000000000 --- a/.github/actions/setup-benchmark-runner/action.yml +++ /dev/null @@ -1,125 +0,0 @@ -name: Setup benchmark runner -description: Install common tools and build packages needed by benchmark jobs. - -inputs: - install-k6: - description: Whether to install k6 for Rails benchmark jobs. - required: false - default: 'true' - -runs: - using: composite - steps: - - name: Install Bencher CLI - # Pinned: alert-detection heuristics grep stderr for "Alert[s]", - # "threshold violation", and "boundary violation". Those strings - # are not a documented API contract, so upgrading the CLI requires - # re-verifying that the expected phrases still appear in alert output. - uses: bencherdev/bencher@v0.6.2 - - - name: Add tools directory to PATH - shell: bash - run: | - mkdir -p ~/bin - echo "$HOME/bin" >> "$GITHUB_PATH" - - - name: Install libyaml-dev - # Must precede any gem install/load: psych links libyaml dynamically. On warm caches the - # missing-lib failure is silent; cold runs hit it. - shell: bash - run: sudo apt-get install -y libyaml-dev - - - name: Setup k6 - if: inputs.install-k6 == 'true' - uses: grafana/setup-k6-action@v1 - with: - k6-version: ${{ env.K6_VERSION }} - - - name: Setup Ruby - uses: ruby/setup-ruby@v1 - with: - ruby-version: ${{ env.RUBY_VERSION }} - - - name: Get gem home directory - shell: bash - run: echo "GEM_HOME_PATH=$(gem env home)" >> "$GITHUB_ENV" - - - name: Cache foreman gem - id: cache-foreman - uses: actions/cache@v5 - with: - path: ${{ env.GEM_HOME_PATH }} - key: foreman-gem-${{ runner.os }}-ruby-${{ env.RUBY_VERSION }} - - - name: Install foreman - if: steps.cache-foreman.outputs.cache-hit != 'true' - shell: bash - run: gem install foreman - - - name: Setup pnpm - uses: pnpm/action-setup@v6 - with: - cache: true - cache_dependency_path: '**/pnpm-lock.yaml' - run_install: false - - - name: Setup Node - uses: actions/setup-node@v6 - with: - node-version: '22' - - - name: Print system information - shell: bash - run: | - echo "Linux release: " - cat /etc/issue - echo "Current user: " - whoami - echo "Current directory: " - pwd - echo "Ruby version: " - ruby -v - echo "Node version: " - node -v - echo "Pnpm version: " - pnpm --version - echo "Bundler version: " - bundle --version - - - name: Configure CPU pinning and process priority - shell: bash - run: | - NPROC=$(nproc) - echo "Available CPUs: $NPROC" - - if [ "$NPROC" -le 1 ]; then - SERVER_CMD="bin/prod" - BENCH_CMD="ruby" - else - SERVER_CMD="nice -n -20 taskset -c 1-$((NPROC-1)) bin/prod" - BENCH_CMD="nice -n -10 taskset -c 0 ruby" - fi - - echo "SERVER_CMD=$SERVER_CMD" >> "$GITHUB_ENV" - echo "BENCH_CMD=$BENCH_CMD" >> "$GITHUB_ENV" - - if [ -z "$WEB_CONCURRENCY" ]; then - # Clamp to >=1 so single-CPU runners don't end up with WEB_CONCURRENCY=0 (Puma - # would start in single-mode, which the benchmarks aren't tuned for). - WEB_CONCURRENCY=$((NPROC > 1 ? NPROC - 1 : 1)) - echo "WEB_CONCURRENCY=$WEB_CONCURRENCY" >> "$GITHUB_ENV" - echo "WEB_CONCURRENCY (auto): $WEB_CONCURRENCY" - else - echo "WEB_CONCURRENCY (from input): $WEB_CONCURRENCY" - fi - - echo "SERVER_CMD: $SERVER_CMD" - echo "BENCH_CMD: $BENCH_CMD" - - - name: Install Node modules with pnpm - shell: bash - run: pnpm install --frozen-lockfile - - - name: Build workspace packages - shell: bash - run: pnpm run build diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 3e6e6de70e..db7e35e7ff 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -78,7 +78,7 @@ env: REQUEST_TIMEOUT: ${{ github.event.inputs.request_timeout }} CONNECTIONS: ${{ github.event.inputs.connections }} MAX_CONNECTIONS: ${{ github.event.inputs.connections }} - # WEB_CONCURRENCY default is set dynamically to NPROC-1 in "Configure CPU pinning" step + # WEB_CONCURRENCY default is set dynamically to NPROC-1 in "Configure benchmark commands" step WEB_CONCURRENCY: ${{ github.event.inputs.web_concurrency }} RAILS_MAX_THREADS: ${{ github.event.inputs.rails_threads || 3 }} RAILS_MIN_THREADS: ${{ github.event.inputs.rails_threads || 3 }} @@ -88,14 +88,11 @@ jobs: permissions: contents: read actions: read - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 outputs: non_runtime_only: ${{ steps.detect.outputs.non_runtime_only }} - run_core_benchmarks: ${{ steps.benchmark-outputs.outputs.run_core_benchmarks }} - run_pro_benchmarks: ${{ steps.benchmark-outputs.outputs.run_pro_benchmarks }} - run_pro_node_renderer_benchmarks: ${{ steps.benchmark-outputs.outputs.run_pro_node_renderer_benchmarks }} - core_benchmark_matrix: ${{ steps.benchmark-matrices.outputs.core_benchmark_matrix }} - pro_benchmark_matrix: ${{ steps.benchmark-matrices.outputs.pro_benchmark_matrix }} + run_benchmark_suites: ${{ steps.benchmark-matrices.outputs.run_benchmark_suites }} + benchmark_matrix: ${{ steps.benchmark-matrices.outputs.benchmark_matrix }} steps: - uses: actions/checkout@v6 with: @@ -147,48 +144,77 @@ jobs: # Always runs. GitHub Actions evaluates `strategy.matrix` for downstream jobs at # workflow-construction time, before their `if:` gates — so an empty output would # crash `fromJSON('')` even when the consuming job is otherwise skipped. - # Bump shard totals here when a suite's route set outgrows its current shard count. + # Bump shard totals in benchmarks/generate_matrix.rb when a suite's route set + # outgrows its current shard count. # The script is stdlib-only, so the runner's system Ruby is enough (no Bundler/Rails boot). + env: + BENCHMARK_APP_VERSION: ${{ github.event.inputs.app_version || 'both' }} + BENCHMARK_EVENT_NAME: ${{ github.event_name }} + BENCHMARK_ROUTES: ${{ github.event.inputs.routes || '' }} + BENCHMARK_NON_RUNTIME_ONLY: ${{ steps.detect.outputs.non_runtime_only }} + BENCHMARK_PULL_REQUEST_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name || '' }} + BENCHMARK_PULL_REQUEST_LABELS: ${{ github.event_name == 'pull_request' && toJSON(github.event.pull_request.labels.*.name) || '[]' }} + GITHUB_REPOSITORY: ${{ github.repository }} + RUN_CORE_BENCHMARKS: ${{ steps.benchmark-outputs.outputs.run_core_benchmarks || 'false' }} + RUN_PRO_BENCHMARKS: ${{ steps.benchmark-outputs.outputs.run_pro_benchmarks || 'false' }} + RUN_PRO_NODE_RENDERER_BENCHMARKS: ${{ steps.benchmark-outputs.outputs.run_pro_node_renderer_benchmarks || 'false' }} run: | - CORE_BENCHMARK_MATRIX=$(BENCHMARK_TOTAL_SHARDS=1 BENCHMARK_SUITE_NAME=Core BENCHMARK_SUITE_PREFIX=CORE ruby benchmarks/count_benchmark_shards.rb) - PRO_BENCHMARK_MATRIX=$(BENCHMARK_TOTAL_SHARDS=2 BENCHMARK_SUITE_NAME=Pro BENCHMARK_SUITE_PREFIX=PRO ruby benchmarks/count_benchmark_shards.rb) + BENCHMARK_MATRIX=$(ruby benchmarks/generate_matrix.rb) + export BENCHMARK_MATRIX + RUN_BENCHMARK_SUITES=$(ruby -rjson -e 'puts JSON.parse(ENV.fetch("BENCHMARK_MATRIX")).fetch("include").any? { |row| row.fetch("suite_id") != "none" }') { - echo "core_benchmark_matrix<> "$GITHUB_OUTPUT" shell: bash - - benchmark-core: + # Only pay the Bundler setup when benchmarks will run or the scripts + # themselves changed; unrelated PRs keep this job stdlib-only and fast. + # TODO: when fixing #2214 this should switch to the new root Gemfile. + - name: Install libyaml-dev + # Must precede any gem install/load: psych links libyaml dynamically. On warm caches the + # missing-lib failure is silent; cold runs hit it. + if: steps.benchmark-matrices.outputs.run_benchmark_suites == 'true' || steps.detect.outputs.benchmarks_changed == 'true' + run: sudo apt-get install -y libyaml-dev + - name: Setup Ruby + if: steps.benchmark-matrices.outputs.run_benchmark_suites == 'true' || steps.detect.outputs.benchmarks_changed == 'true' + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ env.RUBY_VERSION }} + # The benchmark specs only need rspec, which the root Gemfile provides (it + # delegates to react_on_rails/Gemfile). The setup-bundle action can't manage + # it because there is no committed root Gemfile.lock yet, so install manually. + # TODO(#2214): once a root Gemfile.lock is committed, replace this step with + # the setup-bundle action (working-directory: ".") and delete it. + - name: Install gems for benchmark specs + if: steps.benchmark-matrices.outputs.run_benchmark_suites == 'true' || steps.detect.outputs.benchmarks_changed == 'true' + run: bundle install --jobs 4 --retry 3 + # Validate the benchmark scripts (matrix generation, route discovery) before + # any benchmark runs: if this logic is wrong, the matrix and routes feeding + # the run are wrong, so the results would be invalid. Running it here (rather + # than a separate job) reuses this job's checkout/runner and still gates the + # benchmark job, which needs detect-changes to succeed. + - name: Run benchmark script specs + if: steps.benchmark-matrices.outputs.run_benchmark_suites == 'true' || steps.detect.outputs.benchmarks_changed == 'true' + run: bundle exec rspec benchmarks/spec + + benchmark: needs: detect-changes strategy: fail-fast: false - matrix: ${{ fromJSON(needs.detect-changes.outputs.core_benchmark_matrix) }} - # Run on: push to main, workflow_dispatch, or PRs with 'benchmark' / 'benchmark-core' label. - # The 'full-ci' label is intentionally excluded — it controls test workflows, - # not benchmarks. Use dedicated benchmark labels to trigger perf runs on PRs. - # See https://bencher.dev/docs/how-to/github-actions/#pull-requests for the extra pull_request condition - name: Core benchmarks (shard ${{ matrix.shard_label }}) - if: | - contains(fromJSON('["both", "core_only"]'), github.event.inputs.app_version || 'both') && - needs.detect-changes.outputs.non_runtime_only != 'true' && - ( - github.event_name == 'push' || - needs.detect-changes.outputs.run_core_benchmarks == 'true' || - github.event_name == 'workflow_dispatch' || - ( - github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && - ( - contains(github.event.pull_request.labels.*.name, 'benchmark') || - contains(github.event.pull_request.labels.*.name, 'benchmark-core') - ) - ) - ) - runs-on: ubuntu-latest + matrix: ${{ fromJSON(needs.detect-changes.outputs.benchmark_matrix) }} + # Which events/labels actually select suites is decided in + # benchmarks/generate_matrix.rb (suite_requested_by_event? / suite_selected_by_input?): + # push to main, workflow_dispatch, relevant test-impact changes, or same-repo PRs + # with suite-specific benchmark labels. The 'full-ci' label is intentionally + # excluded — it controls test workflows, not benchmarks. This job just runs + # whatever non-empty matrix that produces; the same-repo PR gating follows + # https://bencher.dev/docs/how-to/github-actions/#pull-requests. + name: ${{ matrix.job_name }} + if: needs.detect-changes.outputs.run_benchmark_suites == 'true' + runs-on: ubuntu-24.04 permissions: contents: read issues: write @@ -196,6 +222,7 @@ jobs: checks: write env: SECRET_KEY_BASE: 'dummy-secret-key-for-ci-testing-not-used-in-production' + REACT_ON_RAILS_PRO_LICENSE: ${{ secrets.REACT_ON_RAILS_PRO_LICENSE_V2 }} BENCHMARK_SHARD_INDEX: ${{ matrix.shard_index }} BENCHMARK_TOTAL_SHARDS: ${{ matrix.shard_total }} BENCHMARK_SHARD_LABEL: ${{ matrix.shard_label }} @@ -206,346 +233,146 @@ jobs: with: persist-credentials: false - - name: Setup benchmark runner - uses: ./.github/actions/setup-benchmark-runner - - - name: Install Ruby Gems for Core dummy app - uses: ./.github/actions/setup-bundle - with: - working-directory: react_on_rails/spec/dummy - ruby-version: ${{ env.RUBY_VERSION }} - - - name: Prepare Core production assets - run: | - set -e - echo "🔨 Building Core production assets..." - cd react_on_rails/spec/dummy - - if ! bin/prod-assets; then - echo "❌ ERROR: Failed to build production assets" - exit 1 - fi - - echo "✅ Production assets built successfully" - - - name: Start Core production server - run: | - set -e - echo "🚀 Starting Core production server..." - cd react_on_rails/spec/dummy - - $SERVER_CMD & - echo "Server started in background" - - echo "⏳ Waiting for server to be ready..." - for i in {1..30}; do - if curl -fsS http://localhost:3001 > /dev/null; then - echo "✅ Server is ready and responding" - exit 0 - fi - echo " Attempt $i/30: Server not ready yet..." - sleep 1 - done - - echo "❌ ERROR: Server failed to start within 30 seconds" - exit 1 + - name: Install Bencher CLI + # Pinned: alert-detection heuristics grep stderr for "Alert[s]", + # "threshold violation", and "boundary violation". Those strings + # are not a documented API contract, so upgrading the CLI requires + # re-verifying that the expected phrases still appear in alert output. + uses: bencherdev/bencher@v0.6.2 - - name: Execute Core benchmark suite - timeout-minutes: 120 + - name: Add tools directory to PATH run: | - set -e - echo "🏃 Running Core benchmark suite..." - - if ! $BENCH_CMD benchmarks/bench.rb; then - echo "❌ ERROR: Benchmark execution failed" - exit 1 - fi + mkdir -p ~/bin + echo "$HOME/bin" >> "$GITHUB_PATH" - echo "Benchmark suite completed successfully" - - - name: Validate Core benchmark results - run: | - set -e - echo "🔍 Validating Core benchmark results..." - - if [ ! -f "bench_results/summary.txt" ]; then - echo "❌ ERROR: benchmark summary file not found" - exit 1 - fi - - if [ ! -f "bench_results/benchmark.json" ]; then - echo "❌ ERROR: benchmark JSON file not found" - exit 1 - fi - - echo "✅ Benchmark results found" - echo "" - echo "📊 Summary:" - column -t -s $'\t' "bench_results/summary.txt" - echo "" - echo "Generated files:" - ls -lh bench_results/ + - name: Install libyaml-dev + # Must precede any gem install/load: psych links libyaml dynamically. On warm caches the + # missing-lib failure is silent; cold runs hit it. + run: sudo apt-get install -y libyaml-dev - - name: Upload Core benchmark results - uses: actions/upload-artifact@v7 - if: always() + - name: Setup k6 + if: matrix.benchmark_tool == 'k6' + uses: grafana/setup-k6-action@v1 with: - name: benchmark-core-results-${{ github.run_number }}-${{ matrix.shard_slug }} - path: bench_results/ - retention-days: 30 - if-no-files-found: warn - - - name: Track Core benchmarks with Bencher - env: - BENCHER_API_TOKEN: ${{ secrets.BENCHER_API_TOKEN }} - BENCHER_REPORT_MARKER: ${{ matrix.report_marker }} - BENCHMARK_SUITE_NAME: ${{ matrix.suite_name }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_BASE_SHA: ${{ github.event.pull_request.base.sha || '' }} - PR_NUMBER: ${{ github.event.pull_request.number || '' }} - run: ruby benchmarks/track_benchmarks.rb - - - name: Core workflow summary - if: always() - run: | - echo "📋 Benchmark Workflow Summary" - echo "====================================" - echo "Suite: Core" - echo "Shard: ${{ matrix.shard_label }}" - echo "Status: ${{ job.status }}" - echo "Run number: ${{ github.run_number }}" - echo "Triggered by: ${{ github.actor }}" - echo "Branch: ${{ github.ref_name }}" - - benchmark-pro: - needs: detect-changes - strategy: - fail-fast: false - matrix: ${{ fromJSON(needs.detect-changes.outputs.pro_benchmark_matrix) }} - # Run on: push to main, workflow_dispatch, or PRs with 'benchmark' / 'benchmark-pro' label. - # The 'full-ci' label is intentionally excluded; use the dedicated - # benchmark labels to trigger perf runs on PRs. - name: Pro benchmarks (shard ${{ matrix.shard_label }}) - if: | - contains(fromJSON('["both", "pro_only", "pro_rails_only"]'), github.event.inputs.app_version || 'both') && - needs.detect-changes.outputs.non_runtime_only != 'true' && - ( - github.event_name == 'push' || - needs.detect-changes.outputs.run_pro_benchmarks == 'true' || - github.event_name == 'workflow_dispatch' || - ( - github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && - ( - contains(github.event.pull_request.labels.*.name, 'benchmark') || - contains(github.event.pull_request.labels.*.name, 'benchmark-pro') - ) - ) - ) - runs-on: ubuntu-latest - permissions: - contents: read - issues: write - pull-requests: write - checks: write - env: - SECRET_KEY_BASE: 'dummy-secret-key-for-ci-testing-not-used-in-production' - REACT_ON_RAILS_PRO_LICENSE: ${{ secrets.REACT_ON_RAILS_PRO_LICENSE_V2 }} - BENCHMARK_SHARD_INDEX: ${{ matrix.shard_index }} - BENCHMARK_TOTAL_SHARDS: ${{ matrix.shard_total }} - BENCHMARK_SHARD_LABEL: ${{ matrix.shard_label }} + k6-version: ${{ env.K6_VERSION }} - steps: - - name: Checkout repository - uses: actions/checkout@v6 + - name: Cache Vegeta binary + id: cache-vegeta + if: matrix.benchmark_tool == 'vegeta' + uses: actions/cache@v5 with: - persist-credentials: false + path: ~/bin/vegeta + key: vegeta-${{ runner.os }}-${{ runner.arch }}-${{ env.VEGETA_VERSION }} - - name: Setup benchmark runner - uses: ./.github/actions/setup-benchmark-runner + - name: Install Vegeta + if: matrix.benchmark_tool == 'vegeta' && steps.cache-vegeta.outputs.cache-hit != 'true' + run: | + ARCH=$(dpkg --print-architecture) + echo "Installing Vegeta v${VEGETA_VERSION} (${ARCH})" + wget -q "https://github.com/tsenart/vegeta/releases/download/v${VEGETA_VERSION}/vegeta_${VEGETA_VERSION}_linux_${ARCH}.tar.gz" + tar -xzf "vegeta_${VEGETA_VERSION}_linux_${ARCH}.tar.gz" + mv vegeta ~/bin/ - - name: Install Ruby Gems for Pro dummy app - uses: ./.github/actions/setup-bundle + - name: Setup Ruby + uses: ruby/setup-ruby@v1 with: - working-directory: react_on_rails_pro/spec/dummy ruby-version: ${{ env.RUBY_VERSION }} - - name: Generate file-system based entrypoints for Pro - run: cd react_on_rails_pro/spec/dummy && bundle exec rake react_on_rails:generate_packs - - - name: Prepare Pro production assets - run: | - set -e - echo "🔨 Building Pro production assets..." - cd react_on_rails_pro/spec/dummy - - if ! bin/prod-assets; then - echo "❌ ERROR: Failed to build production assets" - exit 1 - fi + - name: Get gem home directory + id: gem-home + run: echo "path=$(gem env home)" >> "$GITHUB_OUTPUT" - echo "✅ Production assets built successfully" - - - name: Start Pro production server - run: | - set -e - echo "🚀 Starting Pro production server..." - cd react_on_rails_pro/spec/dummy + - name: Cache foreman gem + id: cache-foreman + uses: actions/cache@v5 + with: + path: ${{ steps.gem-home.outputs.path }} + key: foreman-gem-${{ runner.os }}-${{ runner.arch }}-ruby-${{ env.RUBY_VERSION }} - $SERVER_CMD & - echo "Server started in background" + - name: Install foreman + if: steps.cache-foreman.outputs.cache-hit != 'true' + run: gem install foreman - echo "⏳ Waiting for server to be ready..." - for i in {1..30}; do - if curl -fsS http://localhost:3001 > /dev/null; then - echo "✅ Server is ready and responding" - exit 0 - fi - echo " Attempt $i/30: Server not ready yet..." - sleep 1 - done + - name: Setup pnpm + uses: pnpm/action-setup@v6 + with: + cache: true + cache_dependency_path: '**/pnpm-lock.yaml' + run_install: false - echo "❌ ERROR: Server failed to start within 30 seconds" - exit 1 + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version: '22' - - name: Execute Pro benchmark suite - timeout-minutes: 120 + - name: Print system information run: | - set -e - echo "🏃 Running Pro benchmark suite..." - - if ! PRO=true $BENCH_CMD benchmarks/bench.rb; then - echo "❌ ERROR: Benchmark execution failed" - exit 1 - fi - - echo "Benchmark suite completed successfully" - - - name: Validate Pro benchmark results + echo "Linux release: " + cat /etc/issue + echo "Current user: " + whoami + echo "Current directory: " + pwd + echo "Ruby version: " + ruby -v + echo "Node version: " + node -v + echo "Pnpm version: " + pnpm --version + echo "Bundler version: " + bundle --version + + - name: Configure benchmark commands run: | - set -e - echo "🔍 Validating Pro benchmark results..." - - if [ ! -f "bench_results/summary.txt" ]; then - echo "❌ ERROR: Rails benchmark summary file not found" - exit 1 + NPROC=$(nproc) + echo "Available CPUs: $NPROC" + + if [ "$NPROC" -le 1 ]; then + SERVER_CMD="bin/prod" + BENCH_CMD="ruby" + else + SERVER_CMD="taskset -c 1-$((NPROC-1)) bin/prod" + BENCH_CMD="taskset -c 0 ruby" fi - echo "📊 Rails Benchmark Summary:" - column -t -s $'\t' "bench_results/summary.txt" - echo "" - if [ ! -f "bench_results/benchmark.json" ]; then - echo "❌ ERROR: benchmark JSON file not found" - exit 1 + echo "SERVER_CMD=$SERVER_CMD" >> "$GITHUB_ENV" + echo "BENCH_CMD=$BENCH_CMD" >> "$GITHUB_ENV" + + if [ -z "$WEB_CONCURRENCY" ]; then + # Clamp to >=1 so single-CPU runners don't end up with WEB_CONCURRENCY=0 (Puma + # would start in single-mode, which the benchmarks aren't tuned for). + WEB_CONCURRENCY=$((NPROC > 1 ? NPROC - 1 : 1)) + echo "WEB_CONCURRENCY=$WEB_CONCURRENCY" >> "$GITHUB_ENV" + echo "WEB_CONCURRENCY (auto): $WEB_CONCURRENCY" + else + echo "WEB_CONCURRENCY (from input): $WEB_CONCURRENCY" fi - echo "✅ Benchmark results validated" - echo "" - echo "Generated files:" - ls -lh bench_results/ + echo "SERVER_CMD: $SERVER_CMD" + echo "BENCH_CMD: $BENCH_CMD" - - name: Upload Pro benchmark results - uses: actions/upload-artifact@v7 - if: always() - with: - name: benchmark-pro-results-${{ github.run_number }}-${{ matrix.shard_slug }} - path: bench_results/ - retention-days: 30 - if-no-files-found: warn + - name: Install Node modules with pnpm + run: pnpm install --frozen-lockfile - - name: Track Pro benchmarks with Bencher - env: - BENCHER_API_TOKEN: ${{ secrets.BENCHER_API_TOKEN }} - BENCHER_REPORT_MARKER: ${{ matrix.report_marker }} - BENCHMARK_SUITE_NAME: ${{ matrix.suite_name }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_BASE_SHA: ${{ github.event.pull_request.base.sha || '' }} - PR_NUMBER: ${{ github.event.pull_request.number || '' }} - run: ruby benchmarks/track_benchmarks.rb + - name: Build workspace packages + run: pnpm run build - - name: Pro workflow summary - if: always() - run: | - echo "📋 Benchmark Workflow Summary" - echo "====================================" - echo "Suite: Pro" - echo "Shard: ${{ matrix.shard_label }}" - echo "Status: ${{ job.status }}" - echo "Run number: ${{ github.run_number }}" - echo "Triggered by: ${{ github.actor }}" - echo "Branch: ${{ github.ref_name }}" - - benchmark-pro-node-renderer: - needs: detect-changes - # Run on: push to main, workflow_dispatch, or PRs with 'benchmark', 'benchmark-pro', - # or 'benchmark-pro-node-renderer' label. - # The 'full-ci' label is intentionally excluded; use the dedicated - # benchmark labels to trigger perf runs on PRs. - if: | - contains(fromJSON('["both", "pro_only", "pro_node_renderer_only"]'), github.event.inputs.app_version || 'both') && - needs.detect-changes.outputs.non_runtime_only != 'true' && - ( - github.event_name == 'push' || - needs.detect-changes.outputs.run_pro_node_renderer_benchmarks == 'true' || - github.event_name == 'workflow_dispatch' || - ( - github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && - ( - contains(github.event.pull_request.labels.*.name, 'benchmark') || - contains(github.event.pull_request.labels.*.name, 'benchmark-pro') || - contains(github.event.pull_request.labels.*.name, 'benchmark-pro-node-renderer') - ) - ) - ) - runs-on: ubuntu-latest - permissions: - contents: read - issues: write - pull-requests: write - checks: write - env: - SECRET_KEY_BASE: 'dummy-secret-key-for-ci-testing-not-used-in-production' - REACT_ON_RAILS_PRO_LICENSE: ${{ secrets.REACT_ON_RAILS_PRO_LICENSE_V2 }} - - steps: - - name: Checkout repository - uses: actions/checkout@v6 - with: - persist-credentials: false - - - name: Setup benchmark runner - uses: ./.github/actions/setup-benchmark-runner - with: - install-k6: false - - - name: Cache Vegeta binary - id: cache-vegeta - uses: actions/cache@v5 - with: - path: ~/bin/vegeta - key: vegeta-${{ runner.os }}-${{ runner.arch }}-${{ env.VEGETA_VERSION }} - - - name: Install Vegeta - if: steps.cache-vegeta.outputs.cache-hit != 'true' - run: | - echo "Installing Vegeta v${VEGETA_VERSION}" - wget -q "https://github.com/tsenart/vegeta/releases/download/v${VEGETA_VERSION}/vegeta_${VEGETA_VERSION}_linux_amd64.tar.gz" - tar -xzf "vegeta_${VEGETA_VERSION}_linux_amd64.tar.gz" - mv vegeta ~/bin/ - - - name: Install Ruby Gems for Pro dummy app + - name: Install Ruby Gems for the benchmarked app uses: ./.github/actions/setup-bundle with: - working-directory: react_on_rails_pro/spec/dummy + working-directory: ${{ matrix.app_directory }} ruby-version: ${{ env.RUBY_VERSION }} - name: Generate file-system based entrypoints for Pro - run: cd react_on_rails_pro/spec/dummy && bundle exec rake react_on_rails:generate_packs + if: matrix.generate_packs == 'true' + working-directory: ${{ matrix.app_directory }} + run: bundle exec rake react_on_rails:generate_packs - - name: Prepare Pro production assets + - name: Prepare production assets + working-directory: ${{ matrix.app_directory }} run: | set -e - echo "🔨 Building Pro production assets..." - cd react_on_rails_pro/spec/dummy + echo "🔨 Building ${{ matrix.suite_name }} production assets..." if ! bin/prod-assets; then echo "❌ ERROR: Failed to build production assets" @@ -554,24 +381,21 @@ jobs: echo "✅ Production assets built successfully" - - name: Pre-seed Node Renderer bundle cache - # Stages the just-built bundles into `.node-renderer-bundles/-production/` - # so bench-node-renderer.rb finds them at startup instead of waiting for the - # first SSR request to upload them. - run: | - set -e - echo "🔧 Pre-seeding node renderer bundle cache..." - cd react_on_rails_pro/spec/dummy - RAILS_ENV=production NODE_ENV=production \ - bundle exec rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink - - name: Start Pro node renderer + if: matrix.server_kind == 'node-renderer' + working-directory: ${{ matrix.app_directory }} + env: + RAILS_ENV: production + NODE_ENV: production + RENDERER_LOG_LEVEL: error + RENDERER_PORT: 3800 run: | set -e - echo "🚀 Starting Pro node renderer..." - cd react_on_rails_pro/spec/dummy + echo "🔧 Pre-seeding node renderer bundle cache..." + bundle exec rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink - RENDERER_LOG_LEVEL=error RENDERER_PORT=3800 RAILS_ENV=production NODE_ENV=production node renderer/node-renderer.js & + echo "🚀 Starting ${{ matrix.suite_name }} node renderer..." + node renderer/node-renderer.js & echo "Node renderer started in background" echo "⏳ Waiting for node renderer to be ready..." @@ -587,30 +411,68 @@ jobs: echo "❌ ERROR: Node renderer failed to start within 30 seconds" exit 1 - - name: Execute Pro Node Renderer benchmark suite - timeout-minutes: 30 + - name: Start Rails production server + if: matrix.server_kind == 'rails' + working-directory: ${{ matrix.app_directory }} + run: | + set -e + echo "🚀 Starting ${{ matrix.suite_name }} production server..." + $SERVER_CMD & + echo "Server started in background" + + echo "⏳ Waiting for server to be ready..." + for i in {1..30}; do + if curl -fsS http://localhost:3001 > /dev/null; then + echo "✅ Server is ready and responding" + exit 0 + fi + echo " Attempt $i/30: Server not ready yet..." + sleep 1 + done + + echo "❌ ERROR: Server failed to start within 30 seconds" + exit 1 + + - name: Execute benchmark suite + timeout-minutes: ${{ matrix.benchmark_timeout_minutes }} + # Route matrix values through env so the shell treats them as data, not code + # (GitHub Actions injection hardening), even though they come from our own + # generate_matrix.rb. + env: + SUITE_NAME: ${{ matrix.suite_name }} + PRO_ENV: ${{ matrix.pro_env }} + BENCHMARK_SCRIPT: ${{ matrix.benchmark_script }} run: | set -e - echo "🏃 Running Pro Node Renderer benchmark suite..." + echo "🏃 Running $SUITE_NAME benchmark suite..." - if ! $BENCH_CMD benchmarks/bench-node-renderer.rb; then - echo "❌ ERROR: Node Renderer benchmark execution failed" + if [ "$PRO_ENV" = "true" ]; then + if ! PRO=true $BENCH_CMD "$BENCHMARK_SCRIPT"; then + echo "❌ ERROR: Benchmark execution failed" + exit 1 + fi + elif ! $BENCH_CMD "$BENCHMARK_SCRIPT"; then + echo "❌ ERROR: Benchmark execution failed" exit 1 fi - echo "✅ Node Renderer benchmark suite completed successfully" + echo "✅ Benchmark suite completed successfully" - - name: Validate Pro Node Renderer benchmark results + - name: Validate benchmark results + env: + SUITE_NAME: ${{ matrix.suite_name }} + SUMMARY_FILE: ${{ matrix.summary_file }} + SUMMARY_TITLE: ${{ matrix.summary_title }} run: | set -e - echo "🔍 Validating Pro Node Renderer benchmark results..." + echo "🔍 Validating $SUITE_NAME benchmark results..." - if [ ! -f "bench_results/node_renderer_summary.txt" ]; then - echo "❌ ERROR: Node Renderer benchmark summary file not found" + if [ ! -f "$SUMMARY_FILE" ]; then + echo "❌ ERROR: benchmark summary file not found" exit 1 fi - echo "📊 Node Renderer Benchmark Summary:" - column -t -s $'\t' "bench_results/node_renderer_summary.txt" + echo "📊 $SUMMARY_TITLE:" + column -t -s $'\t' "$SUMMARY_FILE" echo "" if [ ! -f "bench_results/benchmark.json" ]; then @@ -623,32 +485,42 @@ jobs: echo "Generated files:" ls -lh bench_results/ - - name: Upload Pro Node Renderer benchmark results + - name: Upload benchmark results uses: actions/upload-artifact@v7 if: always() with: - name: benchmark-pro-node-renderer-results-${{ github.run_number }} + name: ${{ matrix.artifact_name_prefix }}-${{ github.run_number }}${{ matrix.artifact_name_suffix }} path: bench_results/ retention-days: 30 if-no-files-found: warn - - name: Track Pro Node Renderer benchmarks with Bencher + - name: Track benchmarks with Bencher env: BENCHER_API_TOKEN: ${{ secrets.BENCHER_API_TOKEN }} - BENCHER_REPORT_MARKER: '' - BENCHMARK_SUITE_NAME: Pro Node Renderer + BENCHER_REPORT_MARKER: ${{ matrix.report_marker }} + BENCHMARK_SUITE_NAME: ${{ matrix.bencher_suite_name }} GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_BASE_SHA: ${{ github.event.pull_request.base.sha || '' }} PR_NUMBER: ${{ github.event.pull_request.number || '' }} run: ruby benchmarks/track_benchmarks.rb - - name: Pro Node Renderer workflow summary + - name: Benchmark workflow summary if: always() + # Route context/matrix values through env so the shell treats them as data, + # not code (GitHub Actions injection hardening; ref_name can be attacker-set). + env: + SUITE_NAME: ${{ matrix.suite_name }} + SHARD_LABEL: ${{ matrix.shard_label }} + JOB_STATUS: ${{ job.status }} + RUN_NUMBER: ${{ github.run_number }} + ACTOR: ${{ github.actor }} + REF_NAME: ${{ github.ref_name }} run: | echo "📋 Benchmark Workflow Summary" echo "====================================" - echo "Suite: Pro Node Renderer" - echo "Status: ${{ job.status }}" - echo "Run number: ${{ github.run_number }}" - echo "Triggered by: ${{ github.actor }}" - echo "Branch: ${{ github.ref_name }}" + echo "Suite: $SUITE_NAME" + echo "Shard: $SHARD_LABEL" + echo "Status: $JOB_STATUS" + echo "Run number: $RUN_NUMBER" + echo "Triggered by: $ACTOR" + echo "Branch: $REF_NAME" diff --git a/.github/workflows/lint-js-and-ruby.yml b/.github/workflows/lint-js-and-ruby.yml index 92f22d7540..b63eb7b8a6 100644 --- a/.github/workflows/lint-js-and-ruby.yml +++ b/.github/workflows/lint-js-and-ruby.yml @@ -165,6 +165,12 @@ jobs: env: BUNDLE_GEMFILE: ${{ github.workspace }}/react_on_rails/Gemfile run: cd react_on_rails && bundle exec rubocop + # The benchmarks/ scripts live outside the gem, so the package rubocop run + # above doesn't see them. Lint them from the repo root (root .rubocop.yml). + - name: Lint benchmark scripts + env: + BUNDLE_GEMFILE: ${{ github.workspace }}/react_on_rails/Gemfile + run: bundle exec rubocop benchmarks - name: Validate RBS type signatures env: BUNDLE_GEMFILE: ${{ github.workspace }}/react_on_rails/Gemfile diff --git a/benchmarks/count_benchmark_shards.rb b/benchmarks/count_benchmark_shards.rb deleted file mode 100755 index 0ccd0bd3ae..0000000000 --- a/benchmarks/count_benchmark_shards.rb +++ /dev/null @@ -1,33 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -# Emits the GitHub Actions matrix `include:` payload for one benchmark suite. -# Stdlib-only so the workflow can invoke it with the runner's system Ruby — no -# Bundler, no Rails boot. - -require "json" - -SHARD_TOTAL = Integer(ENV.fetch("BENCHMARK_TOTAL_SHARDS")) -SUITE_NAME = ENV.fetch("BENCHMARK_SUITE_NAME") -SUITE_PREFIX = ENV.fetch("BENCHMARK_SUITE_PREFIX") - -raise "BENCHMARK_TOTAL_SHARDS must be positive (got #{SHARD_TOTAL})" unless SHARD_TOTAL.positive? - -matrix = { - include: Array.new(SHARD_TOTAL) do |index| - shard_number = index + 1 - shard_label = "#{shard_number}/#{SHARD_TOTAL}" - shard_slug = "shard-#{shard_number}-of-#{SHARD_TOTAL}" - - { - shard_index: index, - shard_total: SHARD_TOTAL, - shard_label: shard_label, - shard_slug: shard_slug, - suite_name: "#{SUITE_NAME} (shard #{shard_label})", - report_marker: "" - } - end -} - -puts JSON.generate(matrix) diff --git a/benchmarks/generate_matrix.rb b/benchmarks/generate_matrix.rb new file mode 100755 index 0000000000..15daf829c0 --- /dev/null +++ b/benchmarks/generate_matrix.rb @@ -0,0 +1,221 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Emits the GitHub Actions matrix `include:` payload for benchmark suites. +# Stdlib-only so the workflow can invoke it with the runner's system Ruby — no +# Bundler, no Rails boot. + +require "json" + +MIN_ROUTES_FOR_SHARDING = 10 + +SUITES = [ + { + id: "core", + suite_name: "Core", + suite_prefix: "CORE", + shard_total: 1, + app_versions: %w[both core_only], + run_output: "RUN_CORE_BENCHMARKS", + labels: %w[benchmark benchmark-core], + app_directory: "react_on_rails/spec/dummy", + artifact_name: "benchmark-core-results", + benchmark_tool: "k6", + benchmark_script: "benchmarks/bench.rb", + benchmark_timeout_minutes: 120, + pro_env: false, + generate_packs: false, + server_kind: "rails", + summary_file: "bench_results/summary.txt", + summary_title: "Summary" + }, + { + id: "pro", + suite_name: "Pro", + suite_prefix: "PRO", + shard_total: 2, + app_versions: %w[both pro_only pro_rails_only], + run_output: "RUN_PRO_BENCHMARKS", + labels: %w[benchmark benchmark-pro], + app_directory: "react_on_rails_pro/spec/dummy", + artifact_name: "benchmark-pro-results", + benchmark_tool: "k6", + benchmark_script: "benchmarks/bench.rb", + benchmark_timeout_minutes: 120, + pro_env: true, + generate_packs: true, + server_kind: "rails", + summary_file: "bench_results/summary.txt", + summary_title: "Rails Benchmark Summary" + }, + { + id: "pro-node-renderer", + suite_name: "Pro Node Renderer", + suite_prefix: "PRO_NODE_RENDERER", + shard_total: 1, + app_versions: %w[both pro_only pro_node_renderer_only], + run_output: "RUN_PRO_NODE_RENDERER_BENCHMARKS", + labels: %w[benchmark benchmark-pro benchmark-pro-node-renderer], + app_directory: "react_on_rails_pro/spec/dummy", + artifact_name: "benchmark-pro-node-renderer-results", + benchmark_tool: "vegeta", + report_marker: "", + benchmark_script: "benchmarks/bench-node-renderer.rb", + benchmark_timeout_minutes: 30, + pro_env: false, + generate_packs: true, + server_kind: "node-renderer", + summary_file: "bench_results/node_renderer_summary.txt", + summary_title: "Node Renderer Benchmark Summary" + } +].freeze + +# Every app_version any suite accepts. The workflow_dispatch input is constrained +# to these, but validating guards against a typo or future rename silently +# selecting no suites (which would skip benchmarks while CI stays green). +VALID_APP_VERSIONS = SUITES.flat_map { |suite| suite.fetch(:app_versions) }.uniq.freeze + +def truthy_env?(name) + ENV.fetch(name, "false") == "true" +end + +def pull_request_labels + raw_labels = ENV.fetch("BENCHMARK_PULL_REQUEST_LABELS", "[]") + return [] if raw_labels.empty? + + parsed = JSON.parse(raw_labels) + parsed.is_a?(Array) ? parsed : [] +rescue JSON::ParserError => e + raise "BENCHMARK_PULL_REQUEST_LABELS must be JSON array: #{e.message}" +end + +def suite_requested_by_event?(suite, labels) + event_name = ENV.fetch("BENCHMARK_EVENT_NAME") + + return true if event_name == "push" + return true if event_name == "workflow_dispatch" + return false unless event_name == "pull_request" + + # Fork PRs never run: only honor the change-detection (run_output) or label + # triggers when the PR's head repo is this repo. GITHUB_REPOSITORY is always set + # by Actions, so a blank/forked head repo can't match it. + return false unless ENV.fetch("BENCHMARK_PULL_REQUEST_HEAD_REPO", "") == ENV.fetch("GITHUB_REPOSITORY") + + truthy_env?(suite.fetch(:run_output)) || suite.fetch(:labels).intersect?(labels) +end + +def suite_selected_by_input?(suite) + app_version = ENV.fetch("BENCHMARK_APP_VERSION", "both") + unless VALID_APP_VERSIONS.include?(app_version) + raise "BENCHMARK_APP_VERSION must be one of #{VALID_APP_VERSIONS.join(', ')} (got #{app_version.inspect})" + end + + suite.fetch(:app_versions).include?(app_version) +end + +def suite_enabled?(suite, labels) + suite_selected_by_input?(suite) && suite_requested_by_event?(suite, labels) +end + +def explicit_routes + ENV.fetch("BENCHMARK_ROUTES", "").split(",").map(&:strip).reject(&:empty?) +end + +def shard_total_for_suite(suite) + configured_shard_total = suite.fetch(:shard_total) + explicit_route_count = explicit_routes.length + + return configured_shard_total if explicit_route_count.zero? + return 1 if explicit_route_count < MIN_ROUTES_FOR_SHARDING + + [configured_shard_total, explicit_route_count].min +end + +def suite_rows(suite) + shard_total = shard_total_for_suite(suite) + raise "#{suite.fetch(:id)} shard_total must be positive (got #{shard_total})" unless shard_total.positive? + + Array.new(shard_total) do |index| + shard_number = index + 1 + shard_label = "#{shard_number}/#{shard_total}" + shard_slug = "shard-#{shard_number}-of-#{shard_total}" + suite_name = suite.fetch(:suite_name) + suite_prefix = suite.fetch(:suite_prefix) + + suite_row(suite, suite_name, + shard_index: index, + shard_total: shard_total, + shard_label: shard_label, + report_marker: report_marker(suite, suite_prefix, shard_slug), + artifact_name_suffix: artifact_name_suffix(shard_total, shard_slug)) + end +end + +def suite_row(suite, suite_name, shard_index:, shard_total:, shard_label:, report_marker:, artifact_name_suffix:) + { + suite_id: suite.fetch(:id), + suite_name: suite_name, + job_name: benchmark_job_name(suite_name, shard_total, shard_label), + shard_index: shard_index, + shard_total: shard_total, + shard_label: shard_label, + bencher_suite_name: bencher_suite_name(suite_name, shard_total, shard_label), + report_marker: report_marker, + app_directory: suite.fetch(:app_directory), + artifact_name_prefix: suite.fetch(:artifact_name), + artifact_name_suffix: artifact_name_suffix, + benchmark_tool: suite.fetch(:benchmark_tool), + benchmark_script: suite.fetch(:benchmark_script), + benchmark_timeout_minutes: suite.fetch(:benchmark_timeout_minutes), + pro_env: suite.fetch(:pro_env).to_s, + generate_packs: suite.fetch(:generate_packs).to_s, + server_kind: suite.fetch(:server_kind), + summary_file: suite.fetch(:summary_file), + summary_title: suite.fetch(:summary_title) + } +end + +def benchmark_job_name(suite_name, shard_total, shard_label) + return "#{suite_name} benchmarks" if shard_total == 1 + + "#{suite_name} benchmarks (shard #{shard_label})" +end + +def bencher_suite_name(suite_name, shard_total, shard_label) + return suite_name if shard_total == 1 + + "#{suite_name} (shard #{shard_label})" +end + +def artifact_name_suffix(shard_total, shard_slug) + return "" if shard_total == 1 + + "-#{shard_slug}" +end + +def report_marker(suite, suite_prefix, shard_slug) + suite[:report_marker] || "" +end + +# No suite selected: emit one row so the matrix `include:` stays non-empty and all +# rows share keys. Built from a real suite (any will do — its job is gated off via +# run_benchmark_suites) with the "none" sentinel the gate keys on; the friendlier +# job_name is all that's user-visible, as the skipped job's label. +def empty_matrix_row + suite_rows(SUITES.first).first.merge(suite_id: "none", job_name: "Benchmark suites skipped") +end + +def build_matrix + labels = pull_request_labels + rows = if truthy_env?("BENCHMARK_NON_RUNTIME_ONLY") + [] + else + SUITES.select { |suite| suite_enabled?(suite, labels) }.flat_map { |suite| suite_rows(suite) } + end + + { include: rows.empty? ? [empty_matrix_row] : rows } +end + +# Only emit when run as a script; `require`-ing the file (e.g. from specs) just +# loads the helpers above without printing. +puts JSON.generate(build_matrix) if __FILE__ == $PROGRAM_NAME diff --git a/benchmarks/spec/benchmark_matrix_spec.rb b/benchmarks/spec/benchmark_matrix_spec.rb new file mode 100644 index 0000000000..95ac2a61b3 --- /dev/null +++ b/benchmarks/spec/benchmark_matrix_spec.rb @@ -0,0 +1,192 @@ +# frozen_string_literal: true + +require_relative "spec_helper" +require_relative "../generate_matrix" + +# These specs pin the gating/sharding/naming logic in generate_matrix.rb that +# decides which benchmark jobs run and how Bencher attributes their results. The +# script reads everything from ENV, so each example sets the env the workflow +# would pass (see .github/workflows/benchmark.yml "Set benchmark matrices") and +# asserts on the resulting matrix `include:` rows. +RSpec.describe "benchmark matrix generation" do + include BenchmarkEnvHelper + + # Returns the matrix `include:` rows produced for the given ENV. + def rows_for(env) + with_env(env) { build_matrix }.fetch(:include) + end + + def suite_ids_for(env) + rows_for(env).map { |row| row.fetch(:suite_id) } + end + + describe "the empty/skipped placeholder" do + it "emits a single 'none' row when no suite is selected (fork PR)" do + rows = rows_for( + "BENCHMARK_EVENT_NAME" => "pull_request", + "BENCHMARK_PULL_REQUEST_HEAD_REPO" => "contributor/react_on_rails", + "GITHUB_REPOSITORY" => "shakacode/react_on_rails", + "BENCHMARK_PULL_REQUEST_LABELS" => '["benchmark-pro"]' + ) + + expect(rows.size).to eq(1) + # suite_id "none" is the only load-bearing field (it drives + # run_benchmark_suites); the placeholder is otherwise a real row's shape. + expect(rows.first).to include(suite_id: "none") + end + + it "forces the placeholder for non-runtime-only changes regardless of other inputs" do + expect(suite_ids_for( + "BENCHMARK_EVENT_NAME" => "push", + "BENCHMARK_NON_RUNTIME_ONLY" => "true" + )).to eq(["none"]) + end + end + + describe "event gating" do + it "runs every suite on push (app_version 'both')" do + expect(suite_ids_for( + "BENCHMARK_EVENT_NAME" => "push", + "BENCHMARK_APP_VERSION" => "both" + )).to eq(%w[core pro pro pro-node-renderer]) + end + + it "honors a suite-specific run_output even without labels or push" do + # RUN_PRO_BENCHMARKS gates only the Pro suite; core/node-renderer stay off. + expect(suite_ids_for( + "BENCHMARK_EVENT_NAME" => "pull_request", + "BENCHMARK_PULL_REQUEST_HEAD_REPO" => "shakacode/react_on_rails", + "GITHUB_REPOSITORY" => "shakacode/react_on_rails", + "RUN_PRO_BENCHMARKS" => "true" + )).to eq(%w[pro pro]) + end + + it "selects suites by label intersection on a same-repo PR" do + # benchmark-pro is shared by Pro and Pro-Node-Renderer, but not Core. + expect(suite_ids_for( + "BENCHMARK_EVENT_NAME" => "pull_request", + "BENCHMARK_PULL_REQUEST_HEAD_REPO" => "shakacode/react_on_rails", + "GITHUB_REPOSITORY" => "shakacode/react_on_rails", + "BENCHMARK_PULL_REQUEST_LABELS" => '["benchmark-pro"]' + )).to eq(%w[pro pro pro-node-renderer]) + end + + it "ignores PR labels from a fork (fork-safety guard)" do + expect(suite_ids_for( + "BENCHMARK_EVENT_NAME" => "pull_request", + "BENCHMARK_PULL_REQUEST_HEAD_REPO" => "contributor/react_on_rails", + "GITHUB_REPOSITORY" => "shakacode/react_on_rails", + "BENCHMARK_PULL_REQUEST_LABELS" => '["benchmark"]' + )).to eq(["none"]) + end + end + + describe "app_version input filtering" do + it "restricts a workflow_dispatch to the node-renderer suite for pro_node_renderer_only" do + expect(suite_ids_for( + "BENCHMARK_EVENT_NAME" => "workflow_dispatch", + "BENCHMARK_APP_VERSION" => "pro_node_renderer_only" + )).to eq(%w[pro-node-renderer]) + end + + it "restricts to Core only for core_only" do + expect(suite_ids_for( + "BENCHMARK_EVENT_NAME" => "push", + "BENCHMARK_APP_VERSION" => "core_only" + )).to eq(%w[core]) + end + end + + describe "sharding and naming" do + it "expands the 2-shard Pro suite into per-shard rows" do + # pro_rails_only is exclusive to the Pro suite; pro_only would also pull in + # Pro-Node-Renderer (its app_versions include pro_only). + rows = rows_for( + "BENCHMARK_EVENT_NAME" => "push", + "BENCHMARK_APP_VERSION" => "pro_rails_only" + ) + + expect(rows.map { |row| row.fetch(:shard_label) }).to eq(%w[1/2 2/2]) + expect(rows.map { |row| row.fetch(:job_name) }).to eq( + ["Pro benchmarks (shard 1/2)", "Pro benchmarks (shard 2/2)"] + ) + expect(rows.map { |row| row.fetch(:bencher_suite_name) }).to eq( + ["Pro (shard 1/2)", "Pro (shard 2/2)"] + ) + expect(rows.map { |row| row.fetch(:artifact_name_suffix) }).to eq( + %w[-shard-1-of-2 -shard-2-of-2] + ) + expect(rows.map { |row| row.fetch(:report_marker) }).to eq( + ["", ""] + ) + end + + it "drops the shard suffix and plural naming for a single-shard suite" do + row = rows_for( + "BENCHMARK_EVENT_NAME" => "push", + "BENCHMARK_APP_VERSION" => "core_only" + ).fetch(0) + + expect(row).to include( + job_name: "Core benchmarks", + bencher_suite_name: "Core", + artifact_name_suffix: "", + report_marker: "" + ) + end + + it "uses the explicit report_marker override when a suite defines one" do + row = rows_for( + "BENCHMARK_EVENT_NAME" => "push", + "BENCHMARK_APP_VERSION" => "pro_node_renderer_only" + ).fetch(0) + + expect(row.fetch(:report_marker)).to eq("") + end + end + + describe "explicit-route sharding (BENCHMARK_ROUTES, workflow_dispatch)" do + # Isolate the Pro suite (configured for 2 shards) via pro_rails_only, then vary + # the manual route list to drive shard_total_for_suite. Fewer than 10 routes + # isn't worth parallelizing, so it collapses to one shard. + def pro_shard_labels(routes) + rows_for( + "BENCHMARK_EVENT_NAME" => "workflow_dispatch", + "BENCHMARK_APP_VERSION" => "pro_rails_only", + "BENCHMARK_ROUTES" => routes + ).map { |row| row.fetch(:shard_label) } + end + + it "collapses to a single shard when fewer than 10 routes are given" do + expect(pro_shard_labels("/,/hello,/world")).to eq(%w[1/1]) + end + + it "keeps the configured shard count when at least 10 routes are given" do + routes = (1..10).map { |n| "/r#{n}" }.join(",") + expect(pro_shard_labels(routes)).to eq(%w[1/2 2/2]) + end + + it "ignores blank/whitespace entries and falls back to the configured count" do + expect(pro_shard_labels(" , , ")).to eq(%w[1/2 2/2]) + end + end + + describe "input validation" do + it "raises a descriptive error for malformed label JSON" do + expect do + rows_for( + "BENCHMARK_EVENT_NAME" => "pull_request", + "BENCHMARK_PULL_REQUEST_LABELS" => "not-json" + ) + end.to raise_error(/BENCHMARK_PULL_REQUEST_LABELS must be JSON array/) + end + + it "raises when a suite would produce a non-positive shard count" do + with_env({}) do + expect do + suite_rows({ id: "fake", shard_total: 0 }) + end.to raise_error(/fake shard_total must be positive \(got 0\)/) + end + end + end +end diff --git a/react_on_rails/spec/react_on_rails/benchmark_routes_spec.rb b/benchmarks/spec/benchmark_routes_spec.rb similarity index 98% rename from react_on_rails/spec/react_on_rails/benchmark_routes_spec.rb rename to benchmarks/spec/benchmark_routes_spec.rb index 761951b7b6..db2f356ec0 100644 --- a/react_on_rails/spec/react_on_rails/benchmark_routes_spec.rb +++ b/benchmarks/spec/benchmark_routes_spec.rb @@ -2,7 +2,7 @@ require_relative "spec_helper" require "tmpdir" -require_relative "../../../benchmarks/lib/benchmark_routes" +require_relative "../lib/benchmark_routes" RSpec.describe "benchmark route discovery helpers" do describe "#benchmark_routes_from_rails_routes_output" do diff --git a/benchmarks/spec/spec_helper.rb b/benchmarks/spec/spec_helper.rb new file mode 100644 index 0000000000..cd18f90e2d --- /dev/null +++ b/benchmarks/spec/spec_helper.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# Lightweight spec_helper for the stdlib-only scripts under benchmarks/. +# +# Unlike react_on_rails/spec/react_on_rails/spec_helper.rb, this deliberately +# does NOT load Rails or the react_on_rails gem: the benchmark scripts are plain +# Ruby (require "json" and friends), so the runner only needs rspec. Keep it that +# way — pulling in Rails here would defeat the point of testing these in +# isolation and slow the suite down. + +require "json" + +# Mixin for specs that drive scripts which read all of their input from ENV +# (e.g. generate_matrix.rb). `with_env` swaps in the given vars, clears every +# other benchmark gating key so the host/CI environment can't leak in, and +# always restores the original ENV afterwards. +module BenchmarkEnvHelper + GATING_ENV_KEYS = %w[ + BENCHMARK_EVENT_NAME + BENCHMARK_APP_VERSION + BENCHMARK_NON_RUNTIME_ONLY + BENCHMARK_PULL_REQUEST_LABELS + BENCHMARK_PULL_REQUEST_HEAD_REPO + BENCHMARK_ROUTES + GITHUB_REPOSITORY + RUN_CORE_BENCHMARKS + RUN_PRO_BENCHMARKS + RUN_PRO_NODE_RENDERER_BENCHMARKS + ].freeze + + def with_env(vars) + snapshot = ENV.to_h + GATING_ENV_KEYS.each { |key| ENV.delete(key) } + vars.each { |key, value| ENV[key.to_s] = value.to_s } + yield + ensure + ENV.replace(snapshot) + end +end diff --git a/script/ci-changes-detector b/script/ci-changes-detector index 4d15ea8bc7..14bc81dd9e 100755 --- a/script/ci-changes-detector +++ b/script/ci-changes-detector @@ -58,8 +58,11 @@ if [ -z "${CI:-}" ] && [ "$CURRENT_REF" = "HEAD" ]; then fi if [ -z "$CHANGED_FILES" ]; then + # Don't exit here: with no files the loop below sets nothing, DOCS_ONLY stays + # true, and the docs-only output block emits the full flag set (non_runtime_only + # =true, every run_* false, benchmarks_changed=false). Exiting would instead + # leave every output blank. echo "No changes detected between $BASE_REF and $CURRENT_REF" - exit 0 fi # Initialize flags @@ -68,6 +71,7 @@ LINT_CONFIG_CHANGED=false PRO_LINT_CONFIG_CHANGED=false RUBY_CORE_CHANGED=false RSPEC_CHANGED=false +BENCHMARKS_CHANGED=false SPEC_DUMMY_CHANGED=false JS_CHANGED=false GENERATORS_CHANGED=false @@ -241,6 +245,9 @@ source_file_comment_only_change() { # Analyze each changed file while IFS= read -r file; do + # A here-string yields a single empty line for empty input, so skip blank lines — + # otherwise an empty diff would fall into the catch-all below and trigger every job. + [ -z "$file" ] && continue # *.md and docs/* intentionally overlap (docs/foo.md matches both); *.md wins # because it comes first. Several later branches (generators, Ruby core, # specs) also pair top-level and **/* globs for clarity even though * matches @@ -282,6 +289,19 @@ while IFS= read -r file; do fi ;; + # Benchmark scripts (stdlib-only Ruby under benchmarks/) and their specs. + # These only need rubocop here; their specs run in the benchmark workflow's + # script-validation step (not the gem rspec suite), and they don't need the + # full dummy/integration/Pro suite the catch-all would otherwise trigger. + benchmarks/*|benchmarks/**/*) + DOCS_ONLY=false + if source_file_comment_only_change "$file"; then + SOURCE_COMMENT_ONLY_CHANGED=true + else + BENCHMARKS_CHANGED=true + fi + ;; + # JavaScript/TypeScript source (including tests, scripts, and package files) package.json|pnpm-lock.yaml|packages/react-on-rails/src/*|packages/react-on-rails/src/**/*|packages/react-on-rails/tests/*|packages/react-on-rails/tests/**/*|packages/react-on-rails/scripts/*|packages/react-on-rails/scripts/**/*|packages/react-on-rails/package.json|packages/react-on-rails/tsconfig.json|.github/workflows/package-js-tests.yml) DOCS_ONLY=false @@ -396,6 +416,7 @@ elif [ "$SOURCE_COMMENT_ONLY_CHANGED" = true ] || [ "$PRO_SOURCE_COMMENT_ONLY_CH [ "$PRO_LINT_CONFIG_CHANGED" = false ] && [ "$RUBY_CORE_CHANGED" = false ] && [ "$RSPEC_CHANGED" = false ] && + [ "$BENCHMARKS_CHANGED" = false ] && [ "$SPEC_DUMMY_CHANGED" = false ] && [ "$JS_CHANGED" = false ] && [ "$GENERATORS_CHANGED" = false ] && @@ -430,6 +451,7 @@ if [ "$DOCS_ONLY" = true ]; then "run_pro_tests:false" "run_pro_dummy_tests:false" "run_pro_node_renderer_tests:false" + "benchmarks_changed:false" ) # Output to GITHUB_OUTPUT if in GitHub Actions @@ -468,6 +490,7 @@ echo "Changed file categories:" [ "$RUBY_CORE_CHANGED" = true ] && echo -e "${YELLOW} • Ruby core source code${NC}" [ "$JS_CHANGED" = true ] && echo -e "${YELLOW} • JavaScript/TypeScript code${NC}" [ "$RSPEC_CHANGED" = true ] && echo -e "${YELLOW} • RSpec tests${NC}" +[ "$BENCHMARKS_CHANGED" = true ] && echo -e "${YELLOW} • Benchmark scripts${NC}" [ "$SPEC_DUMMY_CHANGED" = true ] && echo -e "${YELLOW} • Dummy app${NC}" [ "$GENERATORS_CHANGED" = true ] && echo -e "${YELLOW} • Generators${NC}" [ "$PRO_JS_CHANGED" = true ] && echo -e "${YELLOW} • React on Rails Pro JavaScript/TypeScript${NC}" @@ -508,6 +531,14 @@ if [ "$RUBY_CORE_CHANGED" = true ] || [ "$RSPEC_CHANGED" = true ] || [ "$GENERAT RUN_RUBY_TESTS=true fi +# Benchmark scripts live outside the gem: they're linted by rubocop and covered +# by their own specs in the benchmark workflow (not the gem rspec suite), so they +# only need lint here. The benchmarks_changed output drives the benchmark +# workflow's script-validation step. +if [ "$BENCHMARKS_CHANGED" = true ]; then + RUN_LINT=true +fi + if [ "$JS_CHANGED" = true ]; then RUN_JS_TESTS=true fi @@ -588,6 +619,7 @@ if [ -n "${GITHUB_OUTPUT:-}" ]; then echo "run_pro_tests=$RUN_PRO_TESTS" echo "run_pro_dummy_tests=$RUN_PRO_DUMMY_TESTS" echo "run_pro_node_renderer_tests=$RUN_PRO_NODE_RENDERER_TESTS" + echo "benchmarks_changed=$BENCHMARKS_CHANGED" } >> "$GITHUB_OUTPUT" fi @@ -606,7 +638,8 @@ if [ "${CI_JSON_OUTPUT:-}" = "1" ]; then "run_pro_lint": $RUN_PRO_LINT, "run_pro_tests": $RUN_PRO_TESTS, "run_pro_dummy_tests": $RUN_PRO_DUMMY_TESTS, - "run_pro_node_renderer_tests": $RUN_PRO_NODE_RENDERER_TESTS + "run_pro_node_renderer_tests": $RUN_PRO_NODE_RENDERER_TESTS, + "benchmarks_changed": $BENCHMARKS_CHANGED } EOF fi diff --git a/script/ci-changes-detector-test.bash b/script/ci-changes-detector-test.bash index daea7a54c4..3c6edf87bc 100644 --- a/script/ci-changes-detector-test.bash +++ b/script/ci-changes-detector-test.bash @@ -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 benchmarks/lib cat > docs/guide.md <<'DOC' # Guide DOC @@ -138,6 +139,13 @@ module ReactOnRails end end end +RUBY + cat > benchmarks/lib/sample.rb <<'RUBY' +module BenchmarkSample + def self.call + "ok" + end +end RUBY cat > react_on_rails_pro/app/controllers/react_on_rails_pro/rolling_deploy/bundles_controller.rb <<'RUBY' module ReactOnRailsPro @@ -553,6 +561,48 @@ test_core_js_changes_request_e2e() { assert_contains "$out" '"run_e2e_tests": true' "core js output" } +test_benchmark_source_change_lints_and_flags_benchmarks_only() { + setup_repo + write_file_change "benchmarks/generate_matrix.rb" + + local out + out="$(detector_output)" + assert_contains "$out" '"benchmarks_changed": true' "benchmark source output" + assert_contains "$out" '"run_lint": true' "benchmark source output" + # Benchmark scripts have no coverage in the gem suite, dummy app, or Pro stack. + assert_contains "$out" '"run_ruby_tests": false' "benchmark source output" + assert_contains "$out" '"run_dummy_tests": false' "benchmark source output" + assert_contains "$out" '"run_e2e_tests": false' "benchmark source output" + assert_contains "$out" '"run_pro_tests": false' "benchmark source output" +} + +test_benchmark_comment_only_change_is_non_runtime_but_keeps_lint() { + setup_repo + perl -0pi -e 's/module BenchmarkSample/# Describe the sample.\nmodule BenchmarkSample/' \ + benchmarks/lib/sample.rb + commit_change "benchmark comment" + + local out + out="$(detector_output)" + assert_contains "$out" '"non_runtime_only": true' "benchmark comment output" + assert_contains "$out" '"run_lint": true' "benchmark comment output" + assert_contains "$out" '"benchmarks_changed": false' "benchmark comment output" + assert_contains "$out" '"run_ruby_tests": false' "benchmark comment output" +} + +test_empty_diff_skips_everything() { + setup_repo + git commit --allow-empty -m "no file changes" >/dev/null + + local out + out="$(detector_output)" + assert_contains "$out" '"non_runtime_only": true' "empty diff output" + assert_contains "$out" '"run_lint": false' "empty diff output" + assert_contains "$out" '"run_ruby_tests": false' "empty diff output" + assert_contains "$out" '"benchmarks_changed": false' "empty diff output" +} + +run_test test_empty_diff_skips_everything run_test test_docs_changes_are_non_runtime_only run_test test_ruby_comment_only_change_skips_heavy_tests_but_keeps_lint run_test test_ruby_block_comment_only_change_skips_heavy_tests_but_keeps_lint @@ -584,6 +634,8 @@ run_test test_generator_only_changes_do_not_request_e2e run_test test_dummy_app_changes_request_e2e run_test test_core_ruby_changes_request_e2e run_test test_core_js_changes_request_e2e +run_test test_benchmark_source_change_lints_and_flags_benchmarks_only +run_test test_benchmark_comment_only_change_is_non_runtime_but_keeps_lint echo echo "CI changes detector tests: $TESTS_RUN run, $TESTS_FAILED failed"