Address feedback

Author: ibacher

api/src/main/java/org/openmrs/logging/MemoryAppender.java

@@ -177,8 +177,8 @@ private static synchronized ThreadSafeCircularFifoQueue<LogEvent> getBuffer(Stri
 			BUFFERS.put(name, new SoftReference<>(buffer));
 		} else if (buffer.capacity() != bufferSize) {
 			ThreadSafeCircularFifoQueue<LogEvent> newBuffer = new ThreadSafeCircularFifoQueue<>(bufferSize);
-			int messagesToMove = Math.min(buffer.size(), bufferSize);
 			LogEvent[] snapshot = buffer.toArray(new LogEvent[0]);
+			int messagesToMove = Math.min(snapshot.length, bufferSize);
 			int start = Math.max(0, snapshot.length - messagesToMove);
 			for (int i = start; i < snapshot.length; i++) {
 				newBuffer.add(snapshot[i]);

api/src/test/java/org/openmrs/logging/MemoryAppenderTest.java

@@ -298,16 +298,21 @@ void getBuffer_shouldMigrateEventsWhenBufferSizeDecreases() {
 			// Create new appender with same name but smaller buffer size
 			MemoryAppender appender2 = MemoryAppender.newBuilder().setName(appenderName)
 			        .setLayout(PatternLayout.newBuilder().withPattern("%m").build()).setBufferSize(3).build();
-			appender2.start();
-
-			// The new appender should have migrated the 3 most recent events
-			List<String> logLines = appender2.getLogLines();
-			assertThat(logLines, contains("C", "D", "E"));
-			assertThat(appender2.getBufferSize(), equalTo(3));
+			try {
+				appender2.start();
+
+				// The new appender should have migrated the 3 most recent events
+				List<String> logLines = appender2.getLogLines();
+				assertThat(logLines, contains("C", "D", "E"));
+				assertThat(appender2.getBufferSize(), equalTo(3));
+			} finally {
+				appender2.stop();
+			}
 		} finally {
 			migrationLogger.removeAppender(appender1);
 			migrationLogger.setLevel(null);
 			((Logger) LogManager.getRootLogger()).getContext().updateLoggers();
+			appender1.stop();
 		}
 	}
 
@@ -333,16 +338,21 @@ void getBuffer_shouldPreserveAllEventsWhenBufferSizeIncreases() {
 			// Create new appender with same name but larger buffer size
 			MemoryAppender appender2 = MemoryAppender.newBuilder().setName(appenderName)
 			        .setLayout(PatternLayout.newBuilder().withPattern("%m").build()).setBufferSize(10).build();
-			appender2.start();
-
-			// All events should be preserved
-			List<String> logLines = appender2.getLogLines();
-			assertThat(logLines, contains("A", "B", "C"));
-			assertThat(appender2.getBufferSize(), equalTo(10));
+			try {
+				appender2.start();
+
+				// All events should be preserved
+				List<String> logLines = appender2.getLogLines();
+				assertThat(logLines, contains("A", "B", "C"));
+				assertThat(appender2.getBufferSize(), equalTo(10));
+			} finally {
+				appender2.stop();
+			}
 		} finally {
 			migrationLogger.removeAppender(appender1);
 			migrationLogger.setLevel(null);
 			((Logger) LogManager.getRootLogger()).getContext().updateLoggers();
+			appender1.stop();
 		}
 	}
 

api/src/test/java/org/openmrs/logging/OpenmrsConfigurationFactoryTest.java

@@ -14,6 +14,7 @@
 import java.nio.file.Path;
 
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -145,15 +146,15 @@ void getConfigurationFiles_shouldIgnoreUnreadableFiles() throws IOException {
 		Files.createDirectories(configDir);
 		Path unreadable = configDir.resolve("log4j2.xml");
 		Files.writeString(unreadable, "<Configuration/>");
-		unreadable.toFile().setReadable(false);
-
-		openmrsUtilMock.when(OpenmrsUtil::getApplicationDataDirectoryAsFile).thenReturn(tempDir.toFile());
-		openmrsUtilMock.when(() -> OpenmrsUtil.getDirectoryInApplicationDataDirectory("configuration"))
-		        .thenReturn(configDir.toFile());
-
-		OpenmrsConfigurationFactory factory = new OpenmrsConfigurationFactory();
+		Assumptions.assumeTrue(unreadable.toFile().setReadable(false),
+		    "Cannot make file unreadable on this platform; skipping test");
 
 		try {
+			openmrsUtilMock.when(OpenmrsUtil::getApplicationDataDirectoryAsFile).thenReturn(tempDir.toFile());
+			openmrsUtilMock.when(() -> OpenmrsUtil.getDirectoryInApplicationDataDirectory("configuration"))
+			        .thenReturn(configDir.toFile());
+
+			OpenmrsConfigurationFactory factory = new OpenmrsConfigurationFactory();
 			assertThat(factory.getConfigurationFiles(), empty());
 		} finally {
 			unreadable.toFile().setReadable(true);