Skip to content

Commit 87668c1

Browse files
committed
fix(vcr): prevent vcr-data modification in PLAYBACK mode
Fixes three bugs that caused vcr-data files to be modified even when tests were configured with @VCRTest(mode = VCRMode.PLAYBACK): 1. Fixed @nested test class annotation lookup - The VCRExtension was not finding @VCRTest annotations on enclosing classes for nested tests, causing it to fall back to the default PLAYBACK_OR_RECORD mode. Added findVCRTestAnnotation() to walk up the class hierarchy. 2. Added temp directory copy for playback mode - In PLAYBACK mode, the vcr-data directory is now copied to a temp directory before mounting to Docker, preventing any modifications to the source files. 3. Fixed registry writes in playback mode - testSuccessful() and testFailed() now check the mode before writing to the registry, only updating it when recording. Additional changes: - Disabled Redis AOF persistence in playback mode (--appendonly no) - Disabled Redis RDB saves in playback mode (--save "")
1 parent 5a0eda8 commit 87668c1

File tree

2 files changed

+94
-12
lines changed

2 files changed

+94
-12
lines changed

core/src/main/java/com/redis/vl/test/vcr/VCRContext.java

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
package com.redis.vl.test.vcr;
22

33
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
4+
import java.io.IOException;
5+
import java.nio.file.FileVisitResult;
46
import java.nio.file.Files;
57
import java.nio.file.Path;
8+
import java.nio.file.SimpleFileVisitor;
9+
import java.nio.file.StandardCopyOption;
10+
import java.nio.file.attribute.BasicFileAttributes;
611
import java.util.ArrayList;
712
import java.util.List;
813
import java.util.Map;
@@ -132,10 +137,22 @@ public VCRCassetteStore getCassetteStore() {
132137
private void startRedis() {
133138
String redisCommand = buildRedisCommand();
134139

140+
// In playback mode, copy data to temp directory to prevent modifications to source files
141+
Path mountPath = dataDir;
142+
if (!effectiveMode.isRecordMode()) {
143+
try {
144+
mountPath = copyDataToTemp();
145+
} catch (Exception e) {
146+
System.err.println(
147+
"VCR: Failed to copy data to temp directory, using original: " + e.getMessage());
148+
mountPath = dataDir;
149+
}
150+
}
151+
135152
redisContainer =
136153
new GenericContainer<>(DockerImageName.parse(config.redisImage()))
137154
.withExposedPorts(6379)
138-
.withFileSystemBind(dataDir.toAbsolutePath().toString(), "/data", BindMode.READ_WRITE)
155+
.withFileSystemBind(mountPath.toAbsolutePath().toString(), "/data", BindMode.READ_WRITE)
139156
.withCommand(redisCommand);
140157

141158
redisContainer.start();
@@ -150,22 +167,59 @@ private void startRedis() {
150167

151168
private String buildRedisCommand() {
152169
StringBuilder cmd = new StringBuilder("redis-stack-server");
153-
cmd.append(" --appendonly yes");
154-
cmd.append(" --appendfsync everysec");
155170
cmd.append(" --dir /data");
156171
cmd.append(" --dbfilename dump.rdb");
157172

158173
if (effectiveMode.isRecordMode()) {
159-
// Enable periodic saves in record mode
174+
// Enable AOF and periodic saves in record mode
175+
cmd.append(" --appendonly yes");
176+
cmd.append(" --appendfsync everysec");
160177
cmd.append(" --save 60 1 --save 300 10");
161178
} else {
162-
// Disable saves in playback mode for speed
179+
// Disable all persistence in playback mode (read-only)
180+
cmd.append(" --appendonly no");
163181
cmd.append(" --save \"\"");
164182
}
165183

166184
return cmd.toString();
167185
}
168186

187+
/**
188+
* Copies VCR data to a temporary directory to prevent modifications to source files. Used in
189+
* playback mode to ensure cassette files are not modified.
190+
*
191+
* @return path to the temporary directory containing the copied data
192+
* @throws IOException if copying fails
193+
*/
194+
private Path copyDataToTemp() throws IOException {
195+
Path tempDir = Files.createTempDirectory("vcr-playback-");
196+
tempDir.toFile().deleteOnExit();
197+
198+
Files.walkFileTree(
199+
dataDir,
200+
new SimpleFileVisitor<>() {
201+
@Override
202+
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
203+
throws IOException {
204+
Path targetDir = tempDir.resolve(dataDir.relativize(dir));
205+
Files.createDirectories(targetDir);
206+
return FileVisitResult.CONTINUE;
207+
}
208+
209+
@Override
210+
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
211+
throws IOException {
212+
Path targetFile = tempDir.resolve(dataDir.relativize(file));
213+
Files.copy(file, targetFile, StandardCopyOption.REPLACE_EXISTING);
214+
return FileVisitResult.CONTINUE;
215+
}
216+
});
217+
218+
System.out.println(
219+
"VCR: Using temporary copy at " + tempDir + " for playback (read-only protection)");
220+
return tempDir;
221+
}
222+
169223
private void waitForRedis() {
170224
for (int i = 0; i < 30; i++) {
171225
try {

core/src/main/java/com/redis/vl/test/vcr/VCRExtension.java

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public class VCRExtension
7070
@Override
7171
public void beforeAll(ExtensionContext extensionContext) throws Exception {
7272
// Get VCR configuration from @VCRTest annotation
73-
VCRTest config = extensionContext.getRequiredTestClass().getAnnotation(VCRTest.class);
73+
// Walk up the class hierarchy to find the annotation (handles @Nested test classes)
74+
VCRTest config = findVCRTestAnnotation(extensionContext.getRequiredTestClass());
7475

7576
if (config == null) {
7677
// No @VCRTest annotation, use defaults
@@ -208,8 +209,11 @@ public void testSuccessful(ExtensionContext extensionContext) {
208209
return;
209210
}
210211

211-
String testId = getTestId(extensionContext);
212-
context.getRegistry().registerSuccess(testId, context.getCurrentCassetteKeys());
212+
// Only update registry when recording, not in playback mode
213+
if (context.getEffectiveMode().isRecordMode()) {
214+
String testId = getTestId(extensionContext);
215+
context.getRegistry().registerSuccess(testId, context.getCurrentCassetteKeys());
216+
}
213217
}
214218

215219
@Override
@@ -218,11 +222,12 @@ public void testFailed(ExtensionContext extensionContext, Throwable cause) {
218222
return;
219223
}
220224

221-
String testId = getTestId(extensionContext);
222-
context.getRegistry().registerFailure(testId, cause.getMessage());
225+
// Only update registry and delete cassettes when recording
226+
if (context.getEffectiveMode().isRecordMode()) {
227+
String testId = getTestId(extensionContext);
228+
context.getRegistry().registerFailure(testId, cause.getMessage());
223229

224-
// Optionally delete cassettes for failed tests in RECORD mode
225-
if (context.getEffectiveMode() == VCRMode.RECORD) {
230+
// Delete cassettes for failed tests in RECORD mode
226231
context.deleteCassettes(context.getCurrentCassetteKeys());
227232
}
228233
}
@@ -251,6 +256,29 @@ private String getTestId(ExtensionContext ctx) {
251256
return ctx.getRequiredTestClass().getName() + ":" + ctx.getRequiredTestMethod().getName();
252257
}
253258

259+
/**
260+
* Finds the @VCRTest annotation on the given class or any of its enclosing classes. This is
261+
* needed to properly handle @Nested test classes where the annotation is on the outer class.
262+
*
263+
* @param testClass the test class to search
264+
* @return the VCRTest annotation if found, null otherwise
265+
*/
266+
private VCRTest findVCRTestAnnotation(Class<?> testClass) {
267+
Class<?> currentClass = testClass;
268+
269+
// Walk up the class hierarchy (enclosing classes for nested classes)
270+
while (currentClass != null) {
271+
VCRTest annotation = currentClass.getAnnotation(VCRTest.class);
272+
if (annotation != null) {
273+
return annotation;
274+
}
275+
// Check enclosing class for @Nested test classes
276+
currentClass = currentClass.getEnclosingClass();
277+
}
278+
279+
return null;
280+
}
281+
254282
/** Default VCRTest annotation values for when no annotation is present. */
255283
@VCRTest
256284
private static class DefaultVCRTest {

0 commit comments

Comments
 (0)