ci: extract environment-dependent unit tests out of browser E2E matrix jobs#3952
Merged
Conversation
…x jobs e2eTests.yml's Ubuntu_Firefox_Grid/Chrome_Grid/MicrosoftEdge_Grid jobs used GLOBAL_TESTING_SCOPE as an unfiltered "everything except a short blocklist" test selector, so every Mockito-mocked unit test in shaft-engine rode along inside those live-grid JVMs under -DsetParallel=METHODS three times over -- exactly the racing pattern S2 (#3928, PR #3936) band-aided with @test(singleThreaded = true) rather than fixed at the root. Excludes testPackage.unitTests (already homed in pr-gate.yml's unit-tests job) and 35 individually-verified com.shaft.**.internal unit-test classes (no real WebDriver construction, no Docker/BrowserStack/network touchpoints -- confirmed via Mockito/mockConstruction usage and absence of live grid/driver calls) from GLOBAL_TESTING_SCOPE, and adds those same 35 classes to pr-gate.yml's shaft-engine unit-tests leg so they keep running, just in their dedicated non-matrix home instead. Closes #3935 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
MohabMohie
enabled auto-merge (squash)
July 21, 2026 13:56
This was referenced Jul 21, 2026
MohabMohie
added a commit
that referenced
this pull request
Jul 21, 2026
…abilities (#3957) fix: recognize cross-platform absolute mobile_app paths in Appium capabilities File.isAbsolute() is host-OS-specific, so a Windows drive-letter mobile_app path (e.g. C:\Windows\System32\notepad.exe) is misdetected as relative on a Linux JVM and gets mangled by prepending user.dir -- breaking the Appium "windows" platform target when SHAFT runs from a non-Windows CI/build machine. Add an OS-independent isAbsoluteAppPath() check (drive-letter regex or leading "/") ahead of the File.isAbsolute() fallback in PropertyFileManager.getAppiumDesiredCapabilities(), so Windows- and Unix-style absolute paths are left unchanged on any host OS while genuinely relative paths still resolve against user.dir as before. Fixes the "Unit Tests (shaft-engine)" CI failure blocking PR #3952: OptionsManagerCoverageUnitTest.shouldInitializeAppiumCapabilitiesForNativeAndMobileWeb failed on ubuntu-22.04 with the Windows app path silently rewritten to "/home/runner/.../shaft-engine/C:\Windows\System32\notepad.exe". Claude-Session: https://claude.ai/code/session_01NDQpYzvnTbDPYwqDwHQoRz Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
e2eTests.yml'sUbuntu_Firefox_Grid/Ubuntu_Chrome_Grid/Ubuntu_MicrosoftEdge_Gridjobs run-Dtest=${GLOBAL_TESTING_SCOPE}, andGLOBAL_TESTING_SCOPEwas an unfiltered "everything except a short blocklist" selector — so every Mockito-mocked, environment-independent unit test inshaft-enginerode along inside those live-grid JVMs under-DsetParallel=METHODS, three times over (once per browser). This is the structural root cause S2 (E2E Tests grid suites: fix deterministic cross-browser failing unit-test cluster (env-dependent tests inside browser matrix jobs) #3928, PR Fix grid-CI test-isolation and portability bugs (S2) #3936) band-aided with@Test(singleThreaded = true)rather than fixed.com.shaft.**.internalunit-test classes (see list below) plus the wholetestPackage.unitTestspackage toGLOBAL_TESTING_SCOPE's exclusion list.pr-gate.yml's existingunit-testsjob (shaft-engine leg) to run those same 35 classes, so they keep executing — now in their dedicated non-matrix, path-conditioned home instead of a nightly browser-matrix job.testPackage.unitTestsalready ran there.Evidence
Each of the 35 moved classes was checked for real driver/grid/network dependency before moving:
new (Chrome|Firefox|Remote|Edge|Safari)WebDriver(,localhost:4444,docker,BrowserStack.outside comments — the only hits are insideMockito.mockConstruction(...)blocks or string-only property values asserted for branch coverage (e.g.DriverFactoryHelperCoverageUnitTest.closeDriverShouldHandleDockerizedExecutionBranchsetsexecutionAddress("dockerized")as a literal string, never touches real Docker).RemoteGridPreflightUnitTestexplicitly disablesremotePreflightEnabledin@AfterMethodcleanup.ImageProcessingActionsScreenshotBaselineUnitTestusesVisualProcessingProviderRegistry.setProviderForTesting(mock(...)), never the real shaft-visual runtime.AssertionEvidenceReporterTest/FailureTraceReporterTest(flagged as environment-coupling risks by the S2/E2E Tests grid suites: fix deterministic cross-browser failing unit-test cluster (env-dependent tests inside browser matrix jobs) #3928 fix) use a fake in-processWebDriverimplementation (RecordingJavascriptExecutorDriver) and Mockito — no real session.shaft-engine/src/test/java(no ambiguous-Dtest=SimpleNamematches inpr-gate.yml).Classes moved (e2eTests.yml grid jobs → pr-gate.yml unit-tests job)
DriverFactoryHelperAugmentationUnitTest, DriverFactoryHelperCoverageUnitTest, DriverFactoryHelperStorageStateUnitTest, OptionsManagerCoverageUnitTest, RemoteGridPreflightUnitTest, ActionsCoverageUnitTest, ActionsExceptionDetailsUnitTest, ElementInformationCoverageUnitTest, ElementsHelperCoverageUnitTest, AriaSnapshotHelperUnitTest, MobileAccessibilityTreeConverterUnitTest, AnimatedGifManagerCoverageUnitTest, ImageProcessingActionsReportingUnitTest, ImageProcessingActionsScreenshotBaselineUnitTest, ScreenshotHelperCoverageUnitTest, ScreenshotManagerCoverageUnitTest, LocatorHealthReporterUnitTest, RecordManagerCoverageUnitTest, PlaywrightSessionFactoryStorageStateUnitTest, ShardPartitionerUnitTest, UpdateCheckerUnitTest, LightHouseGenerateReportCoverageUnitTest, PropertiesHelperDownloadRetryUnitTest, PropertyFileManagerInternalUnitTest, AllureManagerCoverageUnitTest, BrowserPerformanceExecutionReportUnitTest, FlakeProfilerUnitTest, ProgressBarLoggerCoverageUnitTest, ValidationsHelperCoverageUnitTest, ValidationsHelperNewPatternCoverageUnitTest, VisualValidationsBuilderUnitTest, WebDriverElementValidationsBuilderAriaSnapshotUnitTest, WebDriverElementValidationsBuilderCoverageUnitTest, AssertionEvidenceReporterTest, FailureTraceReporterTest
plus the whole
testPackage.unitTestspackage (already ran inpr-gate.yml; now also excluded from the grid jobs to stop the redundant/racy triple-execution).Out of scope (left in place, not touched)
com.shaft.**.internalclasses not suffixedUnitTest(e.g.HealingActionsIntegrationTest,BrowserStorageStateManagerTest,HttpContractBrowserReplayTest) — not individually verified as environment-independent in this pass; left running in the grid jobs.com.shaft.**.internal.*UnitTestclasses whose names already match an existingGLOBAL_TESTING_SCOPEexclusion (Api/Json/etc., e.g.OpenApiCoverageReporterUnitTest,GoogleTinkApiCoverageUnitTest) — already homed inUbuntu_APIs, untouched.*UnitTestclasses outsideinternal/testPackage.unitTestspackages (e.g.AsyncElementActionsCoverageUnitTest,PlaywrightElementActionsUnitTest,AllureListenerCoverageUnitTest) — same racing symptom, but out of this issue's explicitly-scoped pattern; worth a follow-up if the racing recurs.notify_e2e_tests_failure(PR #3951, S5)Untouched — no job was renamed or removed, only the
GLOBAL_TESTING_SCOPEenv var value andpr-gate.yml's existing-Dtest=string changed. Itsneeds:list still names every real job correctly (verified viayaml.safe_load).Test plan
mvn -pl shaft-engine -am -e test-compile—BUILD SUCCESSpython3 -c "import yaml; yaml.safe_load(...)"on both edited workflow files — parses cleanly, job lists unchangedshaft-engine/src/test/java(no ambiguous match risk inpr-gate.yml's-Dtest=)mvn testgrid run not executed locally per repo policy (never run unscoped forked Maven/TestNG runs) — relies on CI (pr-gate.ymlunit-tests leg + nexte2eTests.ymlnightly run) for live confirmationCloses #3935