[QA] Refactor `FeatureContext.php` — God Class Anti-Pattern
- Dominant language
- Go
- Stars
- 2.1k
- Forks
- 274
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 103
Description
**File:** `tests/acceptance/bootstrap/FeatureContext.php`
**Size:** **3,024 lines** (+ ~10,700 lines from 3 traits: `WebDav`, `Sharing`, `Provisioning`)
## Problem
`FeatureContext.php` is a textbook God Class that violates the **Single Responsibility Principle**. It handles **30+ distinct responsibilities**:
| Responsibility Area | Example Methods |
|---|---|
| URL/server management | `getBaseUrl()`, `usingServer()`, `getBasePath()`, `getOCSPath()` |
| User credential resolution | `getPasswordForUser()`, `getActualPassword()`, `updateUserPassword()` + 11x env-var getters |
| User info (display name, email) | `getDisplayNameForUser()`, `getEmailAddressForUser()` |
| Username replacement | `getActualUsername()`, `usersToBeReplaced()` |
| HTTP requests (5 variants) | `sendingToWithDirectUrl()`, `userSendsHTTPMethodToUrl*()` |
| HTTP status code assertions (4 variants) | `theHTTPStatusCodeShouldBe()`, `ShouldBeOr()`, `ShouldBeBetween()`, `ShouldBeFailure()` |
| Status code array tracking | `pushToLastHttpStatusCodesArray()`, `pushToLastOcsCodesArray()`, `clearStatusCodeArrays()` |
| JSON Schema validation (7 methods) | `assertJsonDocumentMatchesSchema()`, `validateSchemaObject()`, `getJsonSchemaErrors()`, etc. |
| LDAP management (10+ fields/methods) | `getLdap()`, `getLdapBaseDN()`, `getLdapHost()`, `isTestingWithLdap()` |
| Inline code substitution engine | `substituteInLineCodes()` — manages **40+** substitution patterns |
| File system operations | `createLocalFileOfSpecificSize()`, `mkDirOnServer()` |
| Table validation | `verifyTableNodeColumns()`, `verifyTableNodeRows()` |
| Lifecycle hooks (14 methods) | `before()`, `setupLogDir()`, `logScenario()`, `checkScenario()`, etc. |
| status.php interaction | `getStatusPhp()`, `getEditionFromStatus()`, `getProductNameFromStatus()` |
| User/Group ID resolution | `getUserIdByUserName()`, `getGroupIdByGroupName()` |
| XML body generation | `getBodyForOCSRequest()` |
| Expected failures | `isExpectedToFail()` |
## Suggested Extraction Targets
| Extracted Service | Logic to Move |
|---|---|
| **PasswordManager** / **UserCredentialService** | All password env-var getters, `getPasswordForUser()`, `getActualPassword()`, `updateUserPassword()` |
| **UserInfoService** | `getDisplayNameForUser()`, `getEmailAddressForUser()`, `getUserIdByUserName()`, `getGroupIdByGroupName()` |
| **HttpResponseAssertions** | All HTTP status code assertion methods + status code arrays |
| **JsonSchemaValidatorService** | All JSON schema validation logic (~7 methods) |
| **LdapConfigService** | All LDAP fields and methods |
| **InlineCodeSubstitutionService** | The `substituteInLineCodes()` engine |
| **ScenarioLifecycleService** | All `@BeforeScenario` / `@AfterScenario` hook methods |
| **FileManagementService** | File creation, deletion, directory utilities |
| **RequestBuilderService** | `sendingToWithDirectUrl()` and all HTTP method step handlers |
## Acceptance Criteria
- [ ] `FeatureContext.php` reduced to **< 500 lines** — orchestrates other classes instead of doing everything
- [ ] At least 3 service classes extracted (e.g. `PasswordManager`, `LdapConfigService`, `JsonSchemaValidatorService`)
- [ ] All existing Behat tests still pass
- [ ] No duplication with existing `TestHelpers/` directory
Contributor guide
Research direction
Start by reading tests/acceptance/bootstrap/FeatureContext.php and its WebDav, Sharing, and Provisioning traits, then compare the proposed responsibilities with the existing TestHelpers/ directory. Map the extractions and dependencies before changing behavior. Done means FeatureContext.php is under 500 lines, at least three services are extracted, duplication is avoided, and all existing Behat tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- testing
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100