owncloud / owncloud/ocis

[QA] Refactor `FeatureContext.php` — God Class Anti-Pattern

Open
#12,455 1 comment 0 reactions 0 assignees View on GitHub
QA:team
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.