Skip to content

fix: honour bypass_author_approval in integration-branch creation gate#274

Open
eg-ayoub wants to merge 1 commit into
mainfrom
improvement/bypass-author-approval-integration-branches
Open

fix: honour bypass_author_approval in integration-branch creation gate#274
eg-ayoub wants to merge 1 commit into
mainfrom
improvement/bypass-author-approval-integration-branches

Conversation

@eg-ayoub

Copy link
Copy Markdown

Split off from #273 per review feedback so the two changes can be released and reverted independently.

Problem

check_integration_branches decides whether to raise RequestIntegrationBranches (asking the user to send /create_integration_branches) or let the flow proceed to actually create the branches. When the author has not explicitly approved the PR, the gate relied on job.settings.approve alone to detect author consent — meaning /approve unblocked integration-branch creation but the equivalent admin escape hatch /bypass_author_approval did not.

check_approvals (in gitwaterflow/__init__.py) already treats bypass_author_approval as equivalent to author approval; the two functions had simply drifted apart, leaving admins with a UX where a /bypass_author_approval comment first triggers a "please request integration branches" reply instead of moving forward.

Fix

Reuse the existing bypass_author_approval helper from gitwaterflow.utils (which also folds in the per-author pr_author_options bypass) inside check_integration_branches, so both entry points agree on what "author has approved" means.

Tests

  • Update the two existing tests that relied on the previous asymmetric behaviour by explicitly leaving bypass_author_approval un-bypassed — their intent (verifying the "request integration branches" gate) is preserved.
  • Add a new positive test test_creation_integration_branch_by_bypass_author_approval that exercises the fixed path.

`check_integration_branches` decides whether to raise
`RequestIntegrationBranches` (asking the user to send
`/create_integration_branches`) or let the flow proceed to actually
create the branches. When the author has not explicitly approved the
PR, the gate relied on `job.settings.approve` alone to detect author
consent — meaning `/approve` unblocked integration-branch creation but
the equivalent admin escape hatch `/bypass_author_approval` did not.

`check_approvals` (in gitwaterflow/__init__.py) already treats
`bypass_author_approval` as equivalent to author approval; the two
functions had simply drifted apart, leaving admins with a UX where a
`/bypass_author_approval` comment first triggers a
"please request integration branches" reply instead of moving forward.

Reuse the existing `bypass_author_approval` helper from
`gitwaterflow.utils` (which also folds in the per-author
`pr_author_options` bypass) inside `check_integration_branches`, so
both entry points agree on what "author has approved" means.

Update the two existing tests that relied on the previous asymmetric
behaviour by explicitly leaving `bypass_author_approval` un-bypassed —
their intent (verifying the "request integration branches" gate) is
preserved. Add a new positive test that exercises the fixed path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.63%. Comparing base (6ddd9a5) to head (b849931).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #274      +/-   ##
==========================================
+ Coverage   89.62%   89.63%   +0.01%     
==========================================
  Files          81       81              
  Lines       10614    10627      +13     
==========================================
+ Hits         9513     9526      +13     
  Misses       1101     1101              
Flag Coverage Δ
integration 87.25% <100.00%> (+0.01%) ⬆️
tests 87.21% <100.00%> (+0.01%) ⬆️
tests-BuildFailedTest 26.62% <12.50%> (-0.02%) ⬇️
tests-QuickTest 34.13% <12.50%> (-0.03%) ⬇️
tests-RepositoryTests 26.28% <12.50%> (-0.02%) ⬇️
tests-TaskQueueTests 51.31% <18.75%> (-0.05%) ⬇️
tests-TestBertE 65.55% <100.00%> (+0.05%) ⬆️
tests-TestQueueing 53.52% <18.75%> (-0.05%) ⬇️
tests-api-mock 15.31% <0.00%> (-0.02%) ⬇️
tests-noqueue 77.46% <100.00%> (+0.03%) ⬆️
tests-noqueue-BuildFailedTest 26.62% <12.50%> (-0.02%) ⬇️
tests-noqueue-QuickTest 34.13% <12.50%> (-0.03%) ⬇️
tests-noqueue-RepositoryTests 26.28% <12.50%> (-0.02%) ⬇️
tests-noqueue-TaskQueueTests 51.31% <18.75%> (-0.05%) ⬇️
tests-noqueue-TestBertE 61.95% <100.00%> (+0.05%) ⬆️
tests-noqueue-TestQueueing 26.30% <12.50%> (-0.02%) ⬇️
tests-server 28.21% <6.25%> (-0.03%) ⬇️
unittests 43.29% <6.25%> (-0.05%) ⬇️
utests 28.07% <6.25%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant