Skip to content

docs: parallel.md advertises unsupported "tests" mode; ExecutionValidator rejects it with a misleading error #35

Closed

Description

@shossain786

Summary

docs-site/docs/guides/parallel.md documents parallel: tests as a supported TestNG mode, but ExecutionValidator (called unconditionally at suite bootstrap) rejects any value other than none, methods, classes. Any user who follows the guide's own table and sets parallel: tests gets a suite that fails to start, with a misleading error message.

Reproduction

execution:
  mode: local
  parallel: tests
  threadCount: 4

browser:
  name: chrome

timeouts:
  explicit: 10
  pageLoad: 30

Run any TestNG suite with this config.

Expected

Either the suite starts and runs with TestNG's tests-level parallelism (per the docs), or — if tests genuinely isn't supported — the docs shouldn't advertise it.

Actual

FrameworkBootstrap.initialize() calls ExecutionValidator.validate(config.getExecution()) unconditionally on every suite start (src/main/java/com/seleniumboot/lifecycle/FrameworkBootstrap.java:36), before any test runs. ExecutionValidator only accepts none/methods/classes:

// src/main/java/com/seleniumboot/execution/ExecutionValidator.java:14-16
if (!"none".equalsIgnoreCase(parallel)
        && !"methods".equalsIgnoreCase(parallel)
        && !"classes".equalsIgnoreCase(parallel)) {
    throw new IllegalStateException("Parallel execution configuration missing");   // line 17
}

parallel: tests throws IllegalStateException: Parallel execution configuration missing at bootstrap, before a single test method runs.

Docs claiming otherwise

docs-site/docs/guides/parallel.md:88:

TestNG supports several parallel modes. Selenium Boot works with all of them:

followed by a table (lines 90–94) that includes:

Mode Description Recommended
tests Each <test> tag in suite XML runs in a thread For suite-level isolation

The inline config sample at line 20 also lists it: parallel: methods # none (default) | methods | classes | tests.

This is consistent with the repo's own CLAUDE.md Configuration Reference, which lists only parallel: none | methods | classes — the docs-site page is the outlier, not the code.

Bug 2 — misleading error message

The thrown message, "Parallel execution configuration missing", is wrong for this case: the value is present, just unsupported. A user setting parallel: tests is told their config is missing, which sends them looking in the wrong place (e.g. re-checking YAML indentation) instead of realizing the value itself is rejected. The message should name the offending value and list the accepted ones, e.g. Unsupported parallel mode 'tests' — expected one of: none, methods, classes. This should be fixed regardless of which option below is chosen for bug 1.

Bug 3 — smaller doc inaccuracy, same file

docs-site/docs/guides/parallel.md:22:

maxActiveSessions: 4    # semaphore cap — cannot exceed threadCount

Nothing in the code enforces this relationship. DriverManager.getOrInitSemaphore() builds a Semaphore sized purely from execution.getMaxActiveSessions() (src/main/java/com/seleniumboot/driver/DriverManager.java:44-51), with no comparison to threadCount anywhere — I grepped SeleniumBootDefaults, ExecutionValidator, and DriverManager and found no such check. The comment reads as an enforced constraint but is only advice; worth a one-word wording fix (e.g. "should not exceed") separate from bugs 1–2.

Two fix options for bug 1 (maintainer decision, not actioned here)

  • (a) Add "tests" to the accepted values in ExecutionValidator. tests is a legitimate TestNG parallel mode and nothing else in the framework appears to care which mode is used — this looks like the more correct fix, but it's a code change.
  • (b) Delete the tests row from the table and the "works with all of them" claim, and change the inline sample comment to none (default) | methods | classes.

Either way, bug 2 (error message) should be fixed regardless of which option is chosen.

This is a framework code decision, so per the current roadmap it's deferred to the 2026-07-29 review gate — filing this to record the defect, not to propose action now.

Context

This surfaced while writing a public blog post that links to this guide: https://seleniumboot.com/blog/parallel-selenium-tests-java-without-thread-safety-bugs — the post deliberately documents modes as none | methods | classes to avoid repeating the docs page's false claim, which is why the two pages are now inconsistent with each other.

Note: the YAML config keys in this same guide (execution: nesting, missing timeouts: block) were already fixed on 2026-07-23 in commit 400e3f3. This tests-mode issue is separate and still open — not a regression of that fix.

Activity

  1. added
    bugSomething isn't working
    documentationImprovements or additions to documentation
    on Jul 25, 2026
  2. added a commit that references this issue on Jul 27, 2026
    fdd2b63
  3. shossain786 commented on Jul 27, 2026

    @shossain786
    ContributorAuthor

    Status update — reopening. #36 fixed Bug 1 only; Bugs 2 and 3 are still open.

    PR #36 (merged fdd2b63) carried a Fixes #35 reference, which auto-closed this
    issue on merge. That was too broad — the PR was deliberately scoped to the docs
    claim, and two of the three defects filed here are untouched. Reopening so they
    stay visible.

    ✅ Bug 1 — docs advertise parallel: tests — FIXED

    The sweep for #36 found the same false claim in a second file this issue never
    named: docs-site/docs/configuration.md, in both the sample YAML comment and the
    ### parallel reference table. So the configuration reference page — the page a
    user is most likely to consult when picking a value — was independently
    advertising a mode that aborts the suite. Both files now state that
    execution.parallel is validated at bootstrap against none/methods/classes
    and that other values are rejected, rather than silently dropping the row.

    ❌ Bug 2 — misleading error message — OPEN, deferred

    ExecutionValidator:17 still throws "Parallel execution configuration missing"
    for a value that is present but not accepted. This is a code change, and a code
    change does not reach users without a release, so it is deferred rather than
    half-done. The docs fix partly mitigates it: the rejected-value behaviour is now
    written down and therefore searchable.

    ❌ Bug 3 — maxActiveSessions comment — OPEN

    docs-site/docs/guides/parallel.md:22 still reads "cannot exceed threadCount".
    Confirmed still present on master. Nothing enforces that relationship —
    DriverManager.getOrInitSemaphore() sizes the semaphore purely from
    maxActiveSessions, with no comparison to threadCount anywhere — so the comment
    describes an enforced constraint that does not exist. Same class of defect as
    Bug 1 (docs asserting behaviour the code does not implement) and a one-word fix.

    Bug 1's option (a) — adding tests to ExecutionValidator instead — was not
    foreclosed
    by #36. It remains a legitimate choice; #36 makes the docs honest
    about current behaviour, which is true either way.

  4. shossain786 commented on Jul 27, 2026

    @shossain786
    ContributorAuthor

    Implementation note for whoever eventually opens ExecutionValidator — recorded
    so it isn't re-derived. Not a proposal to act now.

    The value never reaches any Selenium Boot logic that cares which mode it is. It is
    handed straight to TestNG's own enum:

    // SuiteExecutionListener:45-48
    XmlSuite.ParallelMode mode =
            XmlSuite.ParallelMode.valueOf(execution.getParallel().toUpperCase());
    xmlSuite.setParallel(mode);

    Every other reader only asks whether the mode is none:

    • FrameworkBootstrap:96 — threadCount == 1 && !"none" (warning only)
    • SuiteExecutionListener:42 — !"none" gate around the block above
    • HtmlReportGenerator:77 — displays the string in the report
    • BrowserMatrixListener:63 — copies it verbatim when cloning config

    Nothing anywhere branches on methods vs classes. So parallel: tests would
    work today if the validator let it through
    — XmlSuite.ParallelMode has TESTS
    as a member, and the ThreadLocal driver isolation is per-thread regardless of what
    level TestNG parallelises at.

    Two consequences worth knowing:

    1. ExecutionValidator:14-16 is a hand-written allowlist duplicating an enum that
      already exists — the two can drift, and they already have.
    2. The same allowlist also rejects instances, another legitimate TestNG mode.
      Nobody has hit it, and it was never documented, so it is not a reported defect —
      but it is the same bug with a different value.

    If this file is ever touched for another reason, delegating validation to
    ParallelMode.valueOf inside a try/catch resolves bugs 1 and 2 together: all real
    TestNG modes become valid, and the catch block can name both the rejected value and
    the valid set from the enum rather than from a list that drifts.

    Deliberately not doing that now. The docs no longer advertise tests (#36), so
    the reachable harm is gone; what remains is a poor error message for a user who
    picks the value independently, which nobody has reported. A code change also cannot
    reach users without a release, and there is no release planned. This is a
    five-minute change to make while already in the file, not a reason to open it.

  5. added a commit that references this issue on Jul 28, 2026
  6. added a commit that references this issue on Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdocumentationImprovements or additions to documentation

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions