Skip to content

Commit c4466a8

Browse files
chrisdutzchrisdutz
committed
test: Increase the tests of the spi-drivers module.
1 parent fdf6e46 commit c4466a8

6 files changed

Lines changed: 959 additions & 5 deletions

File tree

‎plc4j/spi/drivers/pom.xml‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,29 @@
8383
<executions>
8484
<!--
8585
This is the SPI3 driver-base core (ConnectionBase, DriverBase, MessageCodecBase, …).
86-
It is still largely covered by integration tests in the individual driver modules
87-
rather than unit tests here, so it does not yet meet the project-wide coverage rule.
88-
Keep measuring/reporting coverage, but do not fail the build until the SPI3 core has
89-
its own unit tests. TODO: remove this override once coverage reaches the 0.80 minimum.
86+
The connection-lifecycle / receive-loop orchestration (ConnectionBase) and the exhaustive
87+
PlcValue converters on the response classes are exercised by the per-driver integration
88+
tests rather than by unit tests here, so the module does not reach the project-wide 0.80
89+
instruction minimum. We still enforce a realistic floor (catching coverage regressions)
90+
and drop the "every class covered" rule. TODO: raise the minimum towards 0.80 as more
91+
SPI-core unit tests are added.
9092
-->
9193
<execution>
9294
<id>check-coverage</id>
9395
<configuration>
94-
<haltOnFailure>false</haltOnFailure>
96+
<haltOnFailure>true</haltOnFailure>
97+
<rules combine.self="override">
98+
<rule implementation="org.jacoco.maven.RuleConfiguration">
99+
<element>BUNDLE</element>
100+
<limits>
101+
<limit implementation="org.jacoco.report.check.Limit">
102+
<counter>INSTRUCTION</counter>
103+
<value>COVEREDRATIO</value>
104+
<minimum>0.25</minimum>
105+
</limit>
106+
</limits>
107+
</rule>
108+
</rules>
95109
</configuration>
96110
</execution>
97111
</executions>

‎plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/DriverBaseTransportValidationTest.java‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.plc4x.java.spi.drivers;
20+
21+
import org.apache.plc4x.java.spi.config.Configuration;
22+
import org.apache.plc4x.java.spi.transports.api.TransportInstance;
23+
import org.apache.plc4x.java.utils.auditlog.api.AuditLog;
24+
import org.junit.jupiter.api.Nested;
25+
import org.junit.jupiter.api.Test;
26+
27+
import java.util.List;
28+
import java.util.Optional;
29+
30+
import static org.junit.jupiter.api.Assertions.assertFalse;
31+
import static org.junit.jupiter.api.Assertions.assertThrows;
32+
import static org.junit.jupiter.api.Assertions.assertTrue;
33+
34+
/**
35+
* Verifies the SPI-core check that a driver may only be opened over one of the transports it
36+
* declares it supports, plus the {@code allow-unsupported-transport} opt-out.
37+
*
38+
* <p>The tests exercise the guard purely at the {@code DriverBase.getConnection(...)} boundary,
39+
* with no network and no registered transport, by distinguishing two failure messages:</p>
40+
* <ul>
41+
* <li><b>Guard rejection</b> — message contains {@value #GUARD_MARKER} (the driver does not
42+
* support the requested transport).</li>
43+
* <li><b>Pre-existing registered-transport failure</b> — message contains
44+
* {@value #REGISTERED_MARKER} (the transport passed the guard but is not a registered
45+
* transport in this test JVM).</li>
46+
* </ul>
47+
*/
48+
class DriverBaseTransportValidationTest {
49+
50+
/** Substring unique to the driver-supported-transport rejection message. */
51+
private static final String GUARD_MARKER = "is not supported by driver";
52+
/** Substring of the pre-existing "transport not registered at all" failure message. */
53+
private static final String REGISTERED_MARKER = "Unsupported transport";
54+
55+
/**
56+
* Configurable {@link DriverBase} stub: protocol code, optional default transport, and the
57+
* declared supported-transport list are all set per test. It never actually connects.
58+
*/
59+
static final class StubDriver extends DriverBase {
60+
private final String protocolCode;
61+
private final String defaultTransport; // nullable -> no default
62+
private final List<String> supportedTransports; // may be empty
63+
64+
StubDriver(String protocolCode, String defaultTransport, List<String> supportedTransports) {
65+
this.protocolCode = protocolCode;
66+
this.defaultTransport = defaultTransport;
67+
this.supportedTransports = supportedTransports;
68+
}
69+
70+
@Override public String getProtocolCode() { return protocolCode; }
71+
@Override public String getProtocolName() { return protocolCode; }
72+
@Override public Optional<String> getDefaultTransportCode() { return Optional.ofNullable(defaultTransport); }
73+
@Override public List<String> getSupportedTransportCodes() { return supportedTransports; }
74+
@Override protected Class<? extends Configuration> getConfigurationClass() { return Configuration.class; }
75+
@Override protected ConnectionBase<?> getConnection(Configuration c, TransportInstance<?> t, AuditLog a) {
76+
throw new UnsupportedOperationException("test stub does not connect");
77+
}
78+
}
79+
80+
/** A driver that supports exactly one transport, "supported", with no default. */
81+
private static StubDriver singleSupportDriver() {
82+
return new StubDriver("stub", null, List.of("supported"));
83+
}
84+
85+
/** Open the connection, assert it throws, and return the (non-null) exception message. */
86+
private static String messageFromFailedConnect(DriverBase driver, String connectionString) {
87+
Throwable t = assertThrows(Throwable.class, () -> driver.getConnection(connectionString));
88+
String msg = t.getMessage();
89+
assertTrue(msg != null && !msg.isBlank(), "expected a non-empty failure message");
90+
return msg;
91+
}
92+
93+
@Nested
94+
class StrictCheck {
95+
96+
@Test
97+
void unsupportedTransportIsRejectedWithRequestedAndSupportedNamed() {
98+
String msg = messageFromFailedConnect(singleSupportDriver(), "stub:other://host");
99+
assertTrue(msg.contains(GUARD_MARKER), "should be the guard rejection: " + msg);
100+
assertTrue(msg.contains("other"), "message must name the requested transport: " + msg);
101+
assertTrue(msg.contains("supported"), "message must list the supported transport(s): " + msg);
102+
}
103+
104+
@Test
105+
void supportedTransportPassesTheGuard() {
106+
String msg = messageFromFailedConnect(singleSupportDriver(), "stub:supported://host");
107+
assertFalse(msg.contains(GUARD_MARKER), "guard must NOT reject a supported transport: " + msg);
108+
assertTrue(msg.contains(REGISTERED_MARKER), "should reach the registered-transport lookup: " + msg);
109+
}
110+
111+
@Test
112+
void supportedButUnregisteredTransportStillFailsPreExisting() {
113+
StubDriver driver = new StubDriver("stub", null, List.of("ghost"));
114+
String msg = messageFromFailedConnect(driver, "stub:ghost://host");
115+
assertFalse(msg.contains(GUARD_MARKER), "guard passed (ghost is declared supported): " + msg);
116+
assertTrue(msg.contains(REGISTERED_MARKER), "pre-existing unregistered-transport failure: " + msg);
117+
}
118+
}
119+
120+
@Nested
121+
class OptOut {
122+
123+
@Test
124+
void optOutAllowsUnsupportedTransportThroughGuard() {
125+
String msg = messageFromFailedConnect(singleSupportDriver(),
126+
"stub:other://host?allow-unsupported-transport=true");
127+
assertFalse(msg.contains(GUARD_MARKER), "opt-out must skip the guard: " + msg);
128+
}
129+
130+
@Test
131+
void optOutDoesNotBypassRegisteredTransportCheck() {
132+
String msg = messageFromFailedConnect(singleSupportDriver(),
133+
"stub:other://host?allow-unsupported-transport=true");
134+
assertTrue(msg.contains(REGISTERED_MARKER),
135+
"registered-transport lookup must still run under opt-out: " + msg);
136+
}
137+
138+
@Test
139+
void invalidOptOutValueIsTreatedAsStrict() {
140+
String msg = messageFromFailedConnect(singleSupportDriver(),
141+
"stub:other://host?allow-unsupported-transport=notabool");
142+
assertTrue(msg.contains(GUARD_MARKER),
143+
"invalid opt-out value must fall back to strict and reject: " + msg);
144+
}
145+
}
146+
147+
@Nested
148+
class DefaultOnlyDriver {
149+
150+
/** No explicit supported list, but a default transport "supported". */
151+
private StubDriver defaultOnlyDriver() {
152+
return new StubDriver("stub", "supported", List.of());
153+
}
154+
155+
@Test
156+
void defaultTransportExplicitPassesTheGuard() {
157+
String msg = messageFromFailedConnect(defaultOnlyDriver(), "stub:supported://host");
158+
assertFalse(msg.contains(GUARD_MARKER), "default transport must pass the guard: " + msg);
159+
assertTrue(msg.contains(REGISTERED_MARKER), msg);
160+
}
161+
162+
@Test
163+
void defaultTransportOmittedPassesTheGuard() {
164+
String msg = messageFromFailedConnect(defaultOnlyDriver(), "stub://host");
165+
assertFalse(msg.contains(GUARD_MARKER), "omitted code -> default must pass the guard: " + msg);
166+
assertTrue(msg.contains(REGISTERED_MARKER), msg);
167+
}
168+
169+
@Test
170+
void otherTransportRejectedForDefaultOnlyDriver() {
171+
String msg = messageFromFailedConnect(defaultOnlyDriver(), "stub:other://host");
172+
assertTrue(msg.contains(GUARD_MARKER), "non-default transport must be rejected: " + msg);
173+
assertTrue(msg.contains("other"), msg);
174+
}
175+
}
176+
}

0 commit comments

Comments
 (0)