Repository navigation
fix: validate execution.parallel against TestNG's own modes (#35) - #37
Merged
Merged
Conversation
Bug 2 — ExecutionValidator threw "Parallel execution configuration missing" for a value that was present but not on its hand-written allowlist, sending the user hunting for config that was not absent. The allowlist duplicated an enum that already exists, and the two had already drifted: the value is handed straight to XmlSuite.setParallel() downstream, and nothing in the framework branches on methods vs classes — only on whether the mode is "none". So `tests` and `instances` were rejected despite being legitimate TestNG modes that would have worked. Validation now delegates to XmlSuite.ParallelMode, so anything TestNG accepts, Selenium Boot accepts. An unrecognised value names both the rejected value and the valid set, read off the enum rather than a list that can drift again. A null or blank value keeps the old "missing" message, which was always accurate for that case. Also aligned SuiteExecutionListener's own valueOf() to Locale.ROOT. Without it the validator and the consumer normalise differently, and a value that passes validation can still blow up at suite start — the same class of drift this change exists to remove. Bug 3 — parallel.md claimed maxActiveSessions "cannot exceed threadCount". Nothing enforces that; getOrInitSemaphore() sizes the semaphore from maxActiveSessions alone. The same file contradicted the claim twelve lines later. Docs list the two newly-accepted modes, gated with a version note: the docs site deploys from master, and advertising `tests` as valid while v3.2.0 on Central still rejects it would recreate the original bug with the sign flipped. Drop the note when the next release ships. New ExecutionValidatorTest covers all five modes, case-insensitivity, both message defects as regressions, and the threadCount bounds that had no coverage. 494 tests pass (was 480); Docusaurus build green. Fixes #35 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the two defects #35 still had open after #36 fixed the docs half.
Bug 2 — misleading error message (code)
ExecutionValidatorthrew "Parallel execution configuration missing" for a value that was present but not on its hand-written allowlist — sending the user hunting for config that was not absent.The allowlist duplicated
XmlSuite.ParallelMode, an enum that already exists, and the two had already drifted. Per the implementation note on #35: the value is handed straight toXmlSuite.setParallel()downstream, and nothing in the framework branches onmethodsvsclasses— only on whether the mode isnone. Sotestsandinstanceswere rejected despite being legitimate TestNG modes that would have worked.Validation now delegates to the enum, so anything TestNG accepts, Selenium Boot accepts. The rejection message names both the offending value and the valid set, read off the enum rather than a list that can drift again:
A
nullor blank value keeps the old "missing" message, which was always accurate for that case.Also aligned
SuiteExecutionListener's ownvalueOf()toLocale.ROOT. Without it the validator and the consumer normalise differently, and a value that passes validation can still blow up at suite start — the same class of drift this change exists to remove.Bug 3 —
maxActiveSessions(docs)guides/parallel.mdclaimed the semaphore cap "cannot exceed threadCount". Nothing enforces that —DriverManager.getOrInitSemaphore()sizes the semaphore frommaxActiveSessionsalone. The same file contradicted the claim twelve lines later, correctly describingthreadCount: 4withmaxActiveSessions: 2.Version gating — deliberate
The docs site deploys from
master, so listingtestsas valid there while v3.2.0 on Central still rejects it would recreate the original bug with the sign flipped. Both mode tables carry a note scoping the two new values to the next release. Drop the note when that release ships.CHANGELOG.mdis untouched — this repo has no "Unreleased" section convention, and changelog entries are written at version-bump time per the release checklist. Whoever cuts the next release needs to add this, or a behaviour change ships unrecorded.Verification
mvn clean test— 494 tests, 0 failures (was 480). NewExecutionValidatorTestcovers all five modes, case-insensitivity, both message defects as regressions, and thethreadCountbounds, which had no coverage at all.npm run buildindocs-site/— green.Fixes #35
🤖 Generated with Claude Code