Skip to content

[#107] Honoured feature-level tags when starting and stopping the server. - #128

Merged
AlexSkrypnyk merged 1 commit into
mainfrom
feature/feature-tag-hooks
Aug 18, 2026
Merged

[#107] Honoured feature-level tags when starting and stopping the server.#128
AlexSkrypnyk merged 1 commit into
mainfrom
feature/feature-tag-hooks

Conversation

@AlexSkrypnyk

Copy link
Copy Markdown
Member

Closes #107

Summary

PhpServerContext gated its @beforeScenario and @afterScenario hooks on $scope->getScenario()->hasTag(static::TAG), but ScenarioNode::getTags() only returns tags attached directly to the scenario node, not tags inherited from the enclosing feature, so a feature tagged once at the top silently disabled both hooks for every scenario in it. This was reproducible in this repo: tests/behat/features/apiserver.feature tags @apiserver at feature level, so across all 14 scenarios the hooks touched port 8888 (PhpServerContext) only, port 8889 (ApiServerContext) was never started or stopped by the hooks, and the whole suite performed exactly 1 process termination; the API scenarios still passed only because apiIsRunning() lazily calls start(), which masked the problem, and the server was never stopped by the hook, leaking until the next run's start() reclaimed the port. The fix adds a shared isTagged(ScenarioScope $scope) helper that checks both $scope->getScenario()->hasTag() and $scope->getFeature()->hasTag(), since ScenarioScope, the interface both BeforeScenarioScope and AfterScenarioScope extend, exposes both getFeature() and getScenario(), and FeatureNode uses the same TaggedNodeTrait as ScenarioNode so it answers hasTag() the same way. ApiServerContext extends PhpServerContext and inherits both hooks unchanged, so fixing the base class restores correct behaviour for both contexts. After the change, on the same suite, port 8889 goes from 0 hook interactions to 26 (13 scenarios times start and stop) and process terminations go from 1 to 14, with all 14 scenarios and 153 steps still passing. As a side benefit, start() passes a fresh PROCESS_TIMESTAMP each time and the API server keys its state file on that value, so where one server instance and one state file used to span all 13 API scenarios, each scenario now gets its own, making cross-scenario state isolation stronger than before.

Changes

  • Added isTagged(ScenarioScope $scope) to PhpServerContext, checking both the scenario's and the feature's tags via hasTag().
  • Updated beforeScenarioStartServer() and afterScenarioStopServer() to call isTagged($scope) instead of checking $scope->getScenario()->hasTag() directly.
  • Added testIsTagged() to PhpServerContextTest with a 5-case data provider: tagged on the scenario, tagged on the feature, tagged on both, tagged on neither, and carrying only a different server's tag. Verified the test catches the bug: with the feature-tag half of the condition removed, exactly 1 case fails (tagged on the feature) and the other 4 pass.

Before / After

BEFORE - feature-level tag missed
┌──────────────────────────────────────────────────┐
│ Feature: @apiserver                               │
│   Scenario: Queue a JSON response                 │
└──────────────────────────────────────────────────┘
                        │
                        ▼
       scope->getScenario()->hasTag('apiserver')
                        │
                        ▼
                     FALSE   (tag lives on the feature node;
                               ScenarioNode::getTags() does not
                               include it)
                        │
                        ▼
                  guard = FALSE
                        │
                        ▼
      ┌───────────────────────────────────────┐
      │ beforeScenarioStartServer()   SKIPPED  │
      │ afterScenarioStopServer()     SKIPPED  │
      └───────────────────────────────────────┘

      Port 8889 hook interactions ......... 0
      Process terminations (suite-wide) .... 1


AFTER - both nodes consulted
┌──────────────────────────────────────────────────┐
│ Feature: @apiserver                               │
│   Scenario: Queue a JSON response                 │
└──────────────────────────────────────────────────┘
                        │
                        ▼
                  isTagged($scope)
                        │
          ┌─────────────┴─────────────┐
          ▼                           ▼
 scope->getScenario()         scope->getFeature()
   ->hasTag('apiserver')        ->hasTag('apiserver')
          │                           │
        FALSE                       TRUE
          └─────────────┬─────────────┘
                         ▼
                   guard = TRUE
                         │
                         ▼
      ┌───────────────────────────────────────┐
      │ beforeScenarioStartServer()    RUNS    │
      │ afterScenarioStopServer()      RUNS    │
      └───────────────────────────────────────┘

      Port 8889 hook interactions ......... 26
      Process terminations (suite-wide) .... 14

@AlexSkrypnyk AlexSkrypnyk added this to the 2.4 milestone Aug 18, 2026
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 6 minutes

Limit details: You’ve used all 2 included reviews currently available under your plan. You completed 87 included PR reviews in the past 7 days; at that activity level, included reviews refill at 2 reviews per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 82d400a6-d46c-4a97-9ff3-6411f47a030f

📥 Commits

Reviewing files that changed from the base of the PR and between 7b853c9 and 8caeaf5.

📒 Files selected for processing (2)
  • src/DrevOps/BehatPhpServer/PhpServerContext.php
  • tests/phpunit/Unit/PhpServerContextTest.php

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

@github-actions

Copy link
Copy Markdown
Code Coverage Report:
  2026-08-18 08:27:20

 Summary:
  Classes: 20.00% (1/5)
  Methods: 67.57% (25/37)
  Lines:   82.31% (363/441)

DrevOps\BehatPhpServer\ApiServerContext
  Methods:  75.00% ( 9/12)   Lines:  95.90% (117/122)
DrevOps\BehatPhpServer\ApiServer\ApiServer
  Methods:  20.00% ( 1/ 5)   Lines:   7.94% (  5/ 63)
DrevOps\BehatPhpServer\ApiServer\Request
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
DrevOps\BehatPhpServer\ApiServer\Response
  Methods:  66.67% ( 2/ 3)   Lines:  95.12% ( 39/ 41)
DrevOps\BehatPhpServer\PhpServerContext
  Methods:  75.00% (12/16)   Lines:  96.17% (201/209)

@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.02%. Comparing base (7b853c9) to head (8caeaf5).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   84.98%   85.02%   +0.03%     
==========================================
  Files           3        3              
  Lines         433      434       +1     
==========================================
+ Hits          368      369       +1     
  Misses         65       65              

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

@AlexSkrypnyk AlexSkrypnyk added the Needs review Pull request needs a review from assigned developers label Aug 18, 2026
@AlexSkrypnyk
AlexSkrypnyk merged commit 0362d42 into main Aug 18, 2026
20 checks passed
@AlexSkrypnyk
AlexSkrypnyk deleted the feature/feature-tag-hooks branch August 18, 2026 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs review Pull request needs a review from assigned developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Honour feature-level tags when starting and stopping the server

1 participant