Skip to content

Commit 444d85d

Browse files
g31pranjalg31pranjal
authored
Introduce "include" block in YAML testing framework (#3819)
This PR introduces a new block called `IncludeBlock` to the YAML testing framework. This block enables importing blocks from other yamsql file. Major changes in this PR: - Replaces `lineNumber` with a more expressive `Reference` as a way of representing locations in the yamsql file. While the former works well when a single file is executed, the latter is a recursive way of referencing in files with nested includes. - Adds a new `IncludeBlock` that is able to open a new yamsql file and "import" its execution block. - Changes to `YamlExecutionContext` in the error reporting mechanism to benefit from `Reference` structure. One thing to call out explicitly is changes to the semantics of correcting explain and metrics. By principle (and simplicity), it is possible to have all kinds of blocks in included yamsql files, except the `PreambleBlock`. Also, an include yamsql file (which is a normal yamsql file, that may or may not be able to be executed independently), can be included multiple times in the same file or different files. 1. This can lead to a situation in the presence of `explain` config, where when this config is run under different environments (coming from different includer), can have conflicting corrections to explain statements and planner metrics. This is dangerous and little undefined. Correction to `explain` will cause conflicts. However, since the metrics are not saved associated to "include" file, but with the "includer" file, it is safe. 2. Even when the metrics are saved with "includer" file, the key to a query's metric is only on query string, and not the location. This will be an issue when the same query is executed multiple times in the file. This problem was already know but has just become more relevant with this change. The solution for (1) is to factor out the `explain` string of the query to the metrics file - but that will not be developer friendly who usually prefers that "near" the query. Looking at another file for the `explain` string might not be productive. Solution for (2) is simply to have some sort of query location information be included in the metrics file and use it to deduplicate different runs of same query.
1 parent b297cb8 commit 444d85d

65 files changed

Lines changed: 2529 additions & 504 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/CustomYamlConstructor.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.apple.foundationdb.record.util.ServiceLoaderProvider;
2424
import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
2525
import com.apple.foundationdb.relational.util.Assert;
26+
import com.apple.foundationdb.relational.yamltests.block.IncludeBlock;
2627
import com.apple.foundationdb.relational.yamltests.block.PreambleBlock;
2728
import com.apple.foundationdb.relational.yamltests.block.SetupBlock;
2829
import com.apple.foundationdb.relational.yamltests.block.TestBlock;
@@ -59,6 +60,7 @@ public CustomYamlConstructor(LoaderOptions loaderOptions) {
5960
requireLineNumber.add(SetupBlock.SchemaTemplateBlock.SCHEMA_TEMPLATE_BLOCK);
6061
requireLineNumber.add(TransactionSetupsBlock.TRANSACTION_SETUP);
6162
requireLineNumber.add(TestBlock.TEST_BLOCK);
63+
requireLineNumber.add(IncludeBlock.INCLUDE);
6264
// commands
6365
requireLineNumber.add(Command.COMMAND_LOAD_SCHEMA_TEMPLATE);
6466
requireLineNumber.add(Command.COMMAND_SET_SCHEMA_STATE);

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/Matchers.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,19 @@ public static <T> T notNull(@Nullable final T object, @Nonnull final String desc
241241
return null;
242242
}
243243

244+
@SpotBugsSuppressWarnings(value = "NP_NONNULL_RETURN_VIOLATION", justification = "should never happen, fail throws")
245+
@Nonnull
246+
public static Map.Entry<?, ?> onlyEntry(@Nonnull final Object obj, @Nonnull final String desc) {
247+
if (obj instanceof Map) {
248+
final var map = ((Map<?, ?>) obj);
249+
if (map.size() != 1) {
250+
fail(String.format(Locale.ROOT, "Expecting map %s to have a single element, however it has %s elements.", desc, map.size()));
251+
}
252+
return ((Map<?, ?>) obj).entrySet().iterator().next();
253+
}
254+
return fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s.", desc, Map.class.getSimpleName(), obj.getClass().getSimpleName()));
255+
}
256+
244257
@Nonnull
245258
public static Object valueElseKey(@Nonnull final Map.Entry<?, ?> entry) {
246259
if (isNull(entry.getKey()) && isNull(entry.getValue())) {

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlExecutionContext.java‎

Lines changed: 175 additions & 138 deletions
Large diffs are not rendered by default.

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlReference.java‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
/*
2+
* YamlReference.java
3+
*
4+
* This source file is part of the FoundationDB open source project
5+
*
6+
* Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
7+
*
8+
* Licensed under the Apache License, Version 2.0 (the "License");
9+
* you may not use this file except in compliance with the License.
10+
* You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*/
20+
21+
package com.apple.foundationdb.relational.yamltests;
22+
23+
import com.apple.foundationdb.relational.util.Assert;
24+
import com.apple.foundationdb.tuple.Tuple;
25+
import com.google.common.collect.ImmutableList;
26+
27+
import javax.annotation.Nonnull;
28+
import javax.annotation.Nullable;
29+
import java.util.Objects;
30+
import java.util.function.Supplier;
31+
32+
/**
33+
* This represents a location in a YAMSQL file.
34+
*/
35+
public class YamlReference implements Comparable<YamlReference> {
36+
@Nonnull
37+
private final YamlResource resource;
38+
private final int lineNumber;
39+
@Nonnull
40+
private final Supplier<Tuple> tupleSupplier;
41+
42+
43+
private YamlReference(@Nonnull final YamlResource resource, int lineNumber) {
44+
this.resource = resource;
45+
this.lineNumber = lineNumber;
46+
this.tupleSupplier = () -> resource.tupleSupplier.get().add(lineNumber);
47+
}
48+
49+
@Nonnull
50+
public YamlResource getResource() {
51+
return resource;
52+
}
53+
54+
public int getLineNumber() {
55+
return lineNumber;
56+
}
57+
58+
/**
59+
* Returns the call stack of this {@link YamlReference}.
60+
* @return a list of locations in the call stack of this {@link YamlReference}.
61+
*/
62+
public ImmutableList<YamlReference> getCallStack() {
63+
final var builder = ImmutableList.<YamlReference>builder();
64+
var current = this;
65+
while (current != null) {
66+
builder.add(current);
67+
current = current.getResource().parentRef;
68+
}
69+
return builder.build();
70+
}
71+
72+
public YamlResource newResource(@Nonnull String path) {
73+
return new YamlResource(this, path);
74+
}
75+
76+
@Override
77+
@Nonnull
78+
public String toString() {
79+
return (getResource().parentRef == null ? "" : getResource().parentRef + " > ") + getResource().getFileName() + ":" + getLineNumber();
80+
}
81+
82+
@Override
83+
public boolean equals(Object object) {
84+
if (object == null) {
85+
return false;
86+
}
87+
if (!(object instanceof YamlReference)) {
88+
return false;
89+
}
90+
final var otherReference = (YamlReference) object;
91+
return getResource().equals(otherReference.getResource()) && getLineNumber() == otherReference.getLineNumber();
92+
}
93+
94+
@Override
95+
public int hashCode() {
96+
return Objects.hash(resource, lineNumber);
97+
}
98+
99+
@Override
100+
public int compareTo(final YamlReference o) {
101+
return tupleSupplier.get().compareTo(o.tupleSupplier.get());
102+
}
103+
104+
/**
105+
* A resource represents path to a YAMSQL file. However, this class also serves the purpose of maintaining the
106+
* parent {@link YamlReference}s that (recursively) provides information about the calling stack that has lead to
107+
* this file being executed.
108+
*/
109+
public static class YamlResource {
110+
@Nullable
111+
private final YamlReference parentRef;
112+
@Nonnull
113+
private final String path;
114+
@Nonnull
115+
private final Supplier<Tuple> tupleSupplier;
116+
117+
private YamlResource(@Nullable final YamlReference parentRef, @Nonnull final String path) {
118+
if (parentRef != null) {
119+
assertNotCyclic(parentRef, path);
120+
}
121+
this.parentRef = parentRef;
122+
this.path = path;
123+
this.tupleSupplier = () -> parentRef == null ? Tuple.from(path) : parentRef.tupleSupplier.get().add(path);
124+
}
125+
126+
public YamlReference withLineNumber(int lineNumber) {
127+
return new YamlReference(this, lineNumber);
128+
}
129+
130+
public boolean isTopLevel() {
131+
return parentRef == null;
132+
}
133+
134+
@Nullable
135+
public YamlReference getParentRef() {
136+
return parentRef;
137+
}
138+
139+
@Nonnull
140+
public String getPath() {
141+
return path;
142+
}
143+
144+
@Override
145+
@Nonnull
146+
public String toString() {
147+
return path + ((parentRef == null) ? "" : " via (" + parentRef + ")");
148+
}
149+
150+
@Override
151+
public boolean equals(Object object) {
152+
if (object == null) {
153+
return false;
154+
}
155+
if (!(object instanceof YamlResource)) {
156+
return false;
157+
}
158+
final var otherResource = (YamlResource) object;
159+
if (parentRef == null) {
160+
if (otherResource.parentRef != null) {
161+
return false;
162+
}
163+
return path.equals(otherResource.path);
164+
}
165+
return parentRef.equals(otherResource.parentRef) && path.equals(otherResource.path);
166+
}
167+
168+
@Override
169+
public int hashCode() {
170+
return Objects.hash(parentRef, path);
171+
}
172+
173+
public static YamlResource base(@Nonnull final String path) {
174+
return new YamlResource(null, path);
175+
}
176+
177+
public String getFileName() {
178+
String fileName;
179+
if (path.contains("/")) {
180+
final String[] split = path.split("/");
181+
fileName = split[split.length - 1];
182+
} else {
183+
fileName = path;
184+
}
185+
return fileName;
186+
}
187+
188+
private static void assertNotCyclic(@Nonnull final YamlReference parentRef, @Nonnull final String path) {
189+
final var asTuple = parentRef.tupleSupplier.get().getItems();
190+
Assert.thatUnchecked(asTuple.stream().noneMatch(path::equals), "Cyclic path detected at: " + parentRef);
191+
}
192+
}
193+
}

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlRunner.java‎

Lines changed: 16 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -21,32 +21,18 @@
2121
package com.apple.foundationdb.relational.yamltests;
2222

2323
import com.apple.foundationdb.relational.api.exceptions.RelationalException;
24-
import com.apple.foundationdb.relational.util.Assert;
25-
import com.apple.foundationdb.relational.util.SpotBugsSuppressWarnings;
26-
import com.apple.foundationdb.relational.yamltests.block.Block;
24+
import com.apple.foundationdb.relational.yamltests.block.IncludeBlock;
2725
import com.apple.foundationdb.relational.yamltests.block.TestBlock;
2826
import org.apache.logging.log4j.LogManager;
2927
import org.apache.logging.log4j.Logger;
30-
import org.junit.jupiter.api.Assertions;
31-
import org.yaml.snakeyaml.DumperOptions;
32-
import org.yaml.snakeyaml.LoaderOptions;
33-
import org.yaml.snakeyaml.Yaml;
34-
import org.yaml.snakeyaml.representer.Representer;
35-
import org.yaml.snakeyaml.resolver.Resolver;
3628

3729
import javax.annotation.Nonnull;
38-
import java.io.FileWriter;
3930
import java.io.IOException;
40-
import java.io.InputStream;
41-
import java.io.PrintWriter;
42-
import java.nio.charset.StandardCharsets;
43-
import java.nio.file.Path;
4431
import java.util.ArrayList;
4532
import java.util.List;
46-
import java.util.Locale;
4733
import java.util.Optional;
4834

49-
@SuppressWarnings({"PMD.GuardLogStatement"}) // It already is, but PMD is confused and reporting error in unrelated locations.
35+
@SuppressWarnings({"PMD.GuardLogStatement", "PMD.AvoidCatchingThrowable"}) // It already is, but PMD is confused and reporting error in unrelated locations.
5036
public final class YamlRunner {
5137

5238
private static final Logger logger = LogManager.getLogger(YamlRunner.class);
@@ -57,47 +43,34 @@ public final class YamlRunner {
5743
static final String TEST_NIGHTLY_REPETITION = "tests.yaml.iterations";
5844

5945
@Nonnull
60-
private final String resourcePath;
46+
private final YamlReference.YamlResource baseResource;
6147

6248
@Nonnull
6349
private final YamlExecutionContext executionContext;
6450

6551
public YamlRunner(@Nonnull String resourcePath, @Nonnull YamlConnectionFactory factory,
6652
@Nonnull final YamlExecutionContext.ContextOptions additionalOptions) throws RelationalException {
67-
this.resourcePath = resourcePath;
68-
this.executionContext = new YamlExecutionContext(resourcePath, factory, additionalOptions);
53+
this.baseResource = YamlReference.YamlResource.base(resourcePath);
54+
this.executionContext = new YamlExecutionContext(baseResource, factory, additionalOptions);
6955
}
7056

7157
public void run() throws Exception {
7258
try {
73-
LoaderOptions loaderOptions = new LoaderOptions();
74-
loaderOptions.setAllowDuplicateKeys(true);
75-
DumperOptions dumperOptions = new DumperOptions();
76-
final var yaml = new Yaml(new CustomYamlConstructor(loaderOptions), new Representer(dumperOptions),
77-
new DumperOptions(), loaderOptions, new Resolver());
78-
59+
final var allBlocks = IncludeBlock.parse(baseResource, executionContext);
7960
final var testBlocks = new ArrayList<TestBlock>();
80-
int blockNumber = 0;
81-
try (var inputStream = getInputStream(resourcePath)) {
82-
for (var doc : yaml.loadAll(inputStream)) {
83-
final var block = Block.parse(doc, blockNumber, executionContext);
84-
logger.debug("⚪️ Executing block at line {} in {}", block.getLineNumber(), resourcePath);
85-
block.execute();
86-
if (block instanceof TestBlock) {
87-
testBlocks.add((TestBlock)block);
88-
}
89-
blockNumber++;
61+
for (final var block: allBlocks) {
62+
if (block instanceof TestBlock) {
63+
testBlocks.add((TestBlock)block);
9064
}
9165
}
92-
for (var block : executionContext.getFinalizeBlocks()) {
93-
logger.debug("⚪️ Executing finalizing block for block at line {} in {}", block.getLineNumber(), resourcePath);
66+
for (final var block: allBlocks) {
67+
logger.debug("⚪️ Executing {} at {}", block.getClass().getSimpleName(), block.getReference());
9468
block.execute();
9569
}
9670
evaluateTestBlockResults(testBlocks);
97-
replaceTestFileIfRequired();
98-
replaceMetricsFileIfRequired();
71+
executionContext.replaceFilesIfRequired();
9972
} catch (RelationalException | IOException e) {
100-
logger.error("‼️ running test file '{}' was not successful", resourcePath, e);
73+
logger.error("‼️ running test file '{}' was not successful", baseResource.getPath(), e);
10174
throw e;
10275
}
10376
}
@@ -117,52 +90,18 @@ private void evaluateTestBlockResults(List<TestBlock> testBlocks) {
11790
logger.info("🟢 TestBlock {}/{} runs successfully", i + 1, testBlocks.size());
11891
} else {
11992
RuntimeException failureInBlock = maybeFailure.get();
120-
logger.error("🔴 TestBlock {}/{} (starting at line {}) fails", i + 1, testBlocks.size(), block.getLineNumber());
93+
logger.error("🔴 TestBlock {}/{} ({}) fails", i + 1, testBlocks.size(), block.getReference());
12194
logger.error("--------------------------------------------------------------------------------------------------------------");
12295
logger.error("Error:", failureInBlock);
12396
logger.error("--------------------------------------------------------------------------------------------------------------");
12497
failure = failure == null ? failureInBlock : failure;
12598
}
12699
}
127100
if (failure != null) {
128-
logger.error("⚠️ Some TestBlocks in {} do not pass. ", resourcePath);
101+
logger.error("⚠️ Some TestBlocks in {} do not pass. ", baseResource.getPath());
129102
throw failure;
130103
} else {
131-
logger.info("🟢 All tests in {} pass successfully.", resourcePath);
132-
}
133-
}
134-
135-
@SpotBugsSuppressWarnings(value = "NP_NONNULL_RETURN_VIOLATION", justification = "should never happen, fail throws")
136-
@Nonnull
137-
private static InputStream getInputStream(@Nonnull final String resourcePath) throws RelationalException {
138-
final ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
139-
InputStream inputStream = classLoader.getResourceAsStream(resourcePath);
140-
Assert.notNull(inputStream, String.format(Locale.ROOT, "could not find '%s' in resources bundle", resourcePath));
141-
return inputStream;
142-
}
143-
144-
private void replaceTestFileIfRequired() {
145-
if (executionContext.getEditedFileStream() == null || !executionContext.isDirty()) {
146-
return;
147-
}
148-
try {
149-
try (var writer = new PrintWriter(new FileWriter(Path.of(System.getProperty("user.dir")).resolve(Path.of("src", "test", "resources", resourcePath)).toAbsolutePath().toString(), StandardCharsets.UTF_8))) {
150-
for (var line : executionContext.getEditedFileStream()) {
151-
writer.println(line);
152-
}
153-
}
154-
logger.info("🟢 Source file {} replaced.", resourcePath);
155-
} catch (IOException e) {
156-
logger.error("⚠️ Source file {} could not be replaced with corrected file.", resourcePath);
157-
Assertions.fail(e);
158-
}
159-
}
160-
161-
private void replaceMetricsFileIfRequired() throws RelationalException {
162-
if (!executionContext.isDirtyMetrics()) {
163-
return;
104+
logger.info("🟢 All tests in {} pass successfully.", baseResource.getPath());
164105
}
165-
executionContext.saveMetricsAsBinaryProto();
166-
executionContext.saveMetricsAsYaml();
167106
}
168107
}

0 commit comments

Comments
 (0)