Skip to content

Commit f1a8d31

Browse files
VietND96VietND96
committed
Fix tests
Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent a030566 commit f1a8d31

File tree

4 files changed

+69
-72
lines changed

4 files changed

+69
-72
lines changed

‎java/src/org/openqa/selenium/grid/node/config/NodeOptions.java‎

Lines changed: 38 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -241,17 +241,13 @@ public Map<Capabilities, Collection<SessionFactory>> getSessionFactories(
241241
+ "Issues related to parallel testing with Internet Explored won't be accepted.");
242242
LOG.warning("Double check if enabling 'override-max-sessions' is really needed");
243243
}
244-
// Use node max-sessions for initial driver discovery
245-
int nodeMaxSessions = config.getInt(NODE_SECTION, "max-sessions").orElse(DEFAULT_MAX_SESSIONS);
246-
247-
Map<WebDriverInfo, Collection<SessionFactory>> allDrivers =
248-
discoverDrivers(nodeMaxSessions, factoryFactory);
244+
Map<WebDriverInfo, Collection<SessionFactory>> allDrivers = discoverDrivers(factoryFactory);
249245

250246
ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories =
251247
ImmutableMultimap.builder();
252248

253249
addDriverFactoriesFromConfig(sessionFactories);
254-
addDriverConfigs(factoryFactory, sessionFactories, nodeMaxSessions);
250+
addDriverConfigs(factoryFactory, sessionFactories);
255251
addSpecificDrivers(allDrivers, sessionFactories);
256252
addDetectedDrivers(allDrivers, sessionFactories);
257253

@@ -272,7 +268,7 @@ public int getMaxSessions() {
272268
config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS);
273269

274270
// Always calculate sum of actual driver sessions for consistency
275-
int totalActualSessions = calculateTotalMaxSessionsFromAllDrivers(maxSessions);
271+
int totalActualSessions = calculateTotalMaxSessionsFromAllDrivers();
276272

277273
if (overrideMaxSessions) {
278274
return totalActualSessions;
@@ -367,33 +363,32 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
367363
result.put(displayName, driverMaxSessions);
368364
}
369365
} else {
370-
// When override-max-sessions = false, use CPU-based distribution with explicit overrides
371-
final int sessionsPerDriverConfig;
372-
if (configList.size() > DEFAULT_MAX_SESSIONS) {
373-
sessionsPerDriverConfig = 1;
374-
} else {
375-
sessionsPerDriverConfig = DEFAULT_MAX_SESSIONS / configList.size();
376-
}
366+
// When override-max-sessions = false, use optimized CPU distribution
367+
List<String> driverNames =
368+
configList.stream()
369+
.map(config -> config.get("display-name"))
370+
.collect(Collectors.toList());
371+
Map<String, Integer> sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames);
377372

378373
for (Map<String, String> configMap : configList) {
379374
String displayName = configMap.get("display-name");
380-
int driverMaxSessions = sessionsPerDriverConfig;
375+
int driverMaxSessions = sessionsPerDriver.getOrDefault(displayName, 1);
381376

382377
// Check if driver config has explicit max-sessions within allowed range
383378
if (configMap.containsKey("max-sessions")) {
384379
int explicitMaxSessions =
385380
parseMaxSessionsSafely(
386381
configMap.get("max-sessions"),
387-
sessionsPerDriverConfig,
382+
driverMaxSessions,
388383
"driver config '" + displayName + "' explicit max-sessions");
389-
if (explicitMaxSessions >= 1 && explicitMaxSessions <= sessionsPerDriverConfig) {
384+
if (explicitMaxSessions >= 1 && explicitMaxSessions <= driverMaxSessions) {
390385
driverMaxSessions = explicitMaxSessions;
391386
}
392387
} else {
393388
// Only apply node max-sessions override if driver config doesn't have explicit
394389
// max-sessions
395390
if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) {
396-
if (nodeMaxSessions >= 1 && nodeMaxSessions <= sessionsPerDriverConfig) {
391+
if (nodeMaxSessions >= 1 && nodeMaxSessions <= driverMaxSessions) {
397392
driverMaxSessions = nodeMaxSessions;
398393
}
399394
}
@@ -423,22 +418,25 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
423418
}
424419
} else {
425420
// When override-max-sessions = false, use optimized CPU distribution
426-
Map<WebDriverInfo, Integer> sessionsPerDriver =
427-
calculateOptimizedCpuDistribution(detectedDrivers);
421+
List<String> driverNames =
422+
detectedDrivers.stream()
423+
.map(WebDriverInfo::getDisplayName)
424+
.collect(Collectors.toList());
425+
Map<String, Integer> sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames);
428426

429427
// Check if node max-sessions is explicitly set and within allowed range
430428
if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) {
431429
for (WebDriverInfo info : detectedDrivers) {
432-
int calculatedSessions = sessionsPerDriver.get(info);
430+
int calculatedSessions = sessionsPerDriver.get(info.getDisplayName());
433431
if (nodeMaxSessions >= 1 && nodeMaxSessions <= calculatedSessions) {
434432
result.put(info.getDisplayName(), nodeMaxSessions);
435433
} else {
436434
result.put(info.getDisplayName(), calculatedSessions);
437435
}
438436
}
439437
} else {
440-
for (Map.Entry<WebDriverInfo, Integer> entry : sessionsPerDriver.entrySet()) {
441-
result.put(entry.getKey().getDisplayName(), entry.getValue());
438+
for (Map.Entry<String, Integer> entry : sessionsPerDriver.entrySet()) {
439+
result.put(entry.getKey(), entry.getValue());
442440
}
443441
}
444442
}
@@ -448,65 +446,51 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
448446
return result;
449447
}
450448

451-
private Map<WebDriverInfo, Integer> calculateOptimizedCpuDistribution(List<WebDriverInfo> infos) {
452-
Map<WebDriverInfo, Integer> sessionsPerDriver = new HashMap<>();
449+
private Map<String, Integer> calculateOptimizedCpuDistribution(List<String> driverNames) {
450+
Map<String, Integer> sessionsPerDriver = new HashMap<>();
453451

454452
// First, allocate sessions for constrained drivers (like Safari)
455453
int remainingCores = DEFAULT_MAX_SESSIONS;
456-
List<WebDriverInfo> constrainedDrivers = new ArrayList<>();
457-
List<WebDriverInfo> flexibleDrivers = new ArrayList<>();
454+
List<String> flexibleDrivers = new ArrayList<>();
458455

459-
for (WebDriverInfo info : infos) {
460-
if (info.getMaximumSimultaneousSessions() == 1
461-
&& SINGLE_SESSION_DRIVERS.contains(info.getDisplayName().toLowerCase(Locale.ENGLISH))) {
462-
constrainedDrivers.add(info);
463-
sessionsPerDriver.put(info, 1);
456+
for (String driverName : driverNames) {
457+
if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) {
458+
// Constrained drivers get exactly 1 session
459+
sessionsPerDriver.put(driverName, 1);
464460
remainingCores--;
465461
} else {
466-
flexibleDrivers.add(info);
462+
flexibleDrivers.add(driverName);
467463
}
468464
}
469465

470466
// Then distribute remaining cores among flexible drivers
471-
if (flexibleDrivers.size() > 0 && remainingCores > 0) {
467+
if (!flexibleDrivers.isEmpty() && remainingCores > 0) {
472468
int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size());
473469
int remainderCores = remainingCores % flexibleDrivers.size();
474470

475471
// Distribute base sessions to all flexible drivers
476472
for (int i = 0; i < flexibleDrivers.size(); i++) {
477-
WebDriverInfo info = flexibleDrivers.get(i);
473+
String driverName = flexibleDrivers.get(i);
478474
int sessions = sessionsPerFlexibleDriver;
479475

480476
// Distribute remainder cores to the first 'remainderCores' drivers
481477
if (i < remainderCores) {
482478
sessions++;
483479
}
484480

485-
sessionsPerDriver.put(info, sessions);
481+
sessionsPerDriver.put(driverName, sessions);
486482
}
487-
488-
LOG.log(
489-
Level.FINE,
490-
"Distributed {0} cores among {1} flexible drivers: {2} base sessions each, "
491-
+ "{3} drivers get +1 extra session",
492-
new Object[] {
493-
remainingCores, flexibleDrivers.size(), sessionsPerFlexibleDriver, remainderCores
494-
});
495-
} else if (flexibleDrivers.size() > 0) {
483+
} else if (!flexibleDrivers.isEmpty()) {
496484
// No remaining cores, give each flexible driver 1 session
497-
for (WebDriverInfo info : flexibleDrivers) {
498-
sessionsPerDriver.put(info, 1);
485+
for (String driverName : flexibleDrivers) {
486+
sessionsPerDriver.put(driverName, 1);
499487
}
500-
LOG.log(
501-
Level.FINE,
502-
"No remaining cores available, assigning 1 session to each of {0} flexible drivers",
503-
flexibleDrivers.size());
504488
}
505489

506490
return sessionsPerDriver;
507491
}
508492

509-
private int calculateTotalMaxSessionsFromAllDrivers(int nodeMaxSessions) {
493+
private int calculateTotalMaxSessionsFromAllDrivers() {
510494
Map<String, Integer> actualMaxSessions = calculateActualMaxSessionsPerDriverConfig();
511495
return actualMaxSessions.values().stream().mapToInt(Integer::intValue).sum();
512496
}
@@ -650,8 +634,7 @@ private SessionFactory createSessionFactory(String clazz, Capabilities stereotyp
650634

651635
private void addDriverConfigs(
652636
Function<ImmutableCapabilities, Collection<SessionFactory>> factoryFactory,
653-
ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories,
654-
int maxSessions) {
637+
ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories) {
655638

656639
Multimap<WebDriverInfo, SessionFactory> driverConfigs = HashMultimap.create();
657640

@@ -721,10 +704,6 @@ private void addDriverConfigs(
721704
// Get actual max-sessions per driver config from centralized calculation
722705
Map<String, Integer> actualMaxSessionsPerConfig =
723706
calculateActualMaxSessionsPerDriverConfig();
724-
boolean overrideMaxSessions =
725-
config
726-
.getBool(NODE_SECTION, "override-max-sessions")
727-
.orElse(OVERRIDE_MAX_SESSIONS);
728707

729708
// iterate over driver configs
730709
configList.forEach(
@@ -888,7 +867,7 @@ private void addSpecificDrivers(
888867
}
889868

890869
private Map<WebDriverInfo, Collection<SessionFactory>> discoverDrivers(
891-
int maxSessions, Function<ImmutableCapabilities, Collection<SessionFactory>> factoryFactory) {
870+
Function<ImmutableCapabilities, Collection<SessionFactory>> factoryFactory) {
892871

893872
if (!config.getBool(NODE_SECTION, "detect-drivers").orElse(DEFAULT_DETECT_DRIVERS)) {
894873
return ImmutableMap.of();

‎java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ void shouldNotOverrideMaxSessionsByDefault() {
614614
}
615615

616616
// Verify node max-sessions is within CPU limit (not the overridden value)
617-
assertThat(nodeOptions.getMaxSessions()).isLessThanOrEqualTo(maxRecommendedSessions);
617+
assertThat(nodeOptions.getMaxSessions()).isEqualTo(maxRecommendedSessions);
618618
}
619619

620620
@Test

‎java/test/org/openqa/selenium/grid/router/StressTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
class StressTest {
5757

5858
private final ExecutorService executor =
59-
Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() * 2);
59+
Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() * 4);
6060
private final List<TearDownFixture> tearDowns = new LinkedList<>();
6161
private Server<?> server;
6262
private Browser browser;
@@ -80,7 +80,7 @@ public void setupServers() {
8080
+ "override-max-sessions = true"
8181
+ "\n"
8282
+ "max-sessions = "
83-
+ Runtime.getRuntime().availableProcessors() * 2
83+
+ Runtime.getRuntime().availableProcessors() * 4
8484
+ "\n"
8585
+ "enable-managed-downloads = true")));
8686
tearDowns.add(deployment);
@@ -137,7 +137,7 @@ void multipleSimultaneousSessions() throws Exception {
137137
executor);
138138
}
139139

140-
CompletableFuture.allOf(futures).get(6, MINUTES);
140+
CompletableFuture.allOf(futures).get(7, MINUTES);
141141
}
142142

143143
@Test
@@ -190,6 +190,6 @@ void multipleSimultaneousSessionsTimedOut() throws Exception {
190190
executor);
191191
}
192192

193-
CompletableFuture.allOf(futures).get(6, MINUTES);
193+
CompletableFuture.allOf(futures).get(7, MINUTES);
194194
}
195195
}

‎rb/spec/unit/selenium/server_spec.rb‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,16 @@
2323

2424
module Selenium
2525
describe Server do
26-
let(:mock_process) { instance_double(WebDriver::ChildProcess).as_null_object }
26+
let(:mock_process) do
27+
instance_double(WebDriver::ChildProcess).tap do |mock|
28+
allow(mock).to receive(:start)
29+
allow(mock).to receive(:wait)
30+
allow(mock).to receive(:stop)
31+
allow(mock).to receive(:detach=)
32+
allow(mock).to receive(:io)
33+
allow(mock).to receive(:io=)
34+
end
35+
end
2736
let(:mock_poller) { instance_double(WebDriver::SocketPoller, connected?: true, closed?: true) }
2837
let(:repo) { 'https://api.github.com/repos/seleniumhq/selenium/releases' }
2938
let(:port) { WebDriver::PortProber.above(4444) }
@@ -49,7 +58,8 @@ module Selenium
4958
it 'uses the given jar file and port' do
5059
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
5160
allow(WebDriver::ChildProcess).to receive(:build)
52-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234')
61+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234',
62+
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
5363
.and_return(mock_process)
5464

5565
server = described_class.new('selenium_server_deploy.jar', port: 1234, background: true)
@@ -58,13 +68,15 @@ module Selenium
5868
server.start
5969
expect(File).to have_received(:exist?).with('selenium_server_deploy.jar')
6070
expect(WebDriver::ChildProcess).to have_received(:build)
61-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234')
71+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234',
72+
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
6273
end
6374

6475
it 'waits for the server process by default' do
6576
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
6677
allow(WebDriver::ChildProcess).to receive(:build)
67-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s)
78+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
79+
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
6880
.and_return(mock_process)
6981

7082
server = described_class.new('selenium_server_deploy.jar', port: port)
@@ -75,14 +87,16 @@ module Selenium
7587

7688
expect(File).to have_received(:exist?).with('selenium_server_deploy.jar')
7789
expect(WebDriver::ChildProcess).to have_received(:build)
78-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s)
90+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
91+
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
7992
expect(mock_process).to have_received(:wait)
8093
end
8194

8295
it 'adds additional args' do
8396
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
8497
allow(WebDriver::ChildProcess).to receive(:build)
85-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, 'foo', 'bar')
98+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
99+
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar')
86100
.and_return(mock_process)
87101

88102
server = described_class.new('selenium_server_deploy.jar', port: port, background: true)
@@ -94,7 +108,8 @@ module Selenium
94108
expect(File).to have_received(:exist?).with('selenium_server_deploy.jar')
95109
expect(WebDriver::ChildProcess).to have_received(:build)
96110
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone',
97-
'--port', port.to_s, 'foo', 'bar')
111+
'--port', port.to_s, '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s,
112+
'foo', 'bar')
98113
end
99114

100115
it 'adds additional JAVA options args' do
@@ -105,6 +120,8 @@ module Selenium
105120
'-jar', 'selenium_server_deploy.jar',
106121
'standalone',
107122
'--port', port.to_s,
123+
'--override-max-sessions', 'true',
124+
'--max-sessions', Etc.nprocessors.to_s,
108125
'foo',
109126
'bar')
110127
.and_return(mock_process)
@@ -197,7 +214,8 @@ module Selenium
197214
it 'raises Selenium::Server::Error if the server is not launched within the timeout' do
198215
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
199216
allow(WebDriver::ChildProcess).to receive(:build)
200-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s)
217+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
218+
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
201219
.and_return(mock_process)
202220

203221
poller = instance_double(WebDriver::SocketPoller)

0 commit comments

Comments
 (0)