Skip to content

Commit

Permalink
Fix flaky tests on windows (#222)
Browse files Browse the repository at this point in the history
- Fixes deletion logic
- Stops using deprecated class/method

{patch}

Signed-off-by: Esta Nagy <[email protected]>
  • Loading branch information
nagyesta authored Apr 24, 2024
1 parent 5b1814d commit f853b08
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import com.github.nagyesta.filebarj.io.stream.internal.DoOnCloseInputStream;
import lombok.NonNull;
import org.apache.commons.io.IOUtils;
import org.apache.commons.io.input.CountingInputStream;
import org.apache.commons.io.input.BoundedInputStream;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -40,7 +40,7 @@ public ManifestCipherInputStream(
final var secretKey = EncryptionUtil.byteArrayToAesKey(secretKeyBytes);
crypto = EncryptionUtil.newCipherInputStream(secretKey).decorate(source);
} else {
crypto = new CountingInputStream(source);
crypto = BoundedInputStream.builder().setInputStream(source).get();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,14 @@ private void deleteManifestAndArchiveFilesFromBackupDirectory(@NotNull final Str
.forEach(toDelete::add);
for (final var path : toDelete) {
log.info("Deleting obsolete file: {}", path);
Files.delete(path);
try {
Files.delete(path);
} catch (final IOException e) {
log.warn("Unable to delete file! Will attempt to delete it on exit.", e);
if (Files.exists(path)) {
path.toFile().deleteOnExit();
}
}
}
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void testSetMetadataShouldSetPermissionsOwnerGroupAndTimestampsWhenCalledWithExi
Assertions.assertEquals(metadata.getGroup(), actualMetadata.getGroup());
Assertions.assertEquals(metadata.getLastModifiedUtcEpochSeconds(), actualMetadata.getLastModifiedUtcEpochSeconds());
Assertions.assertEquals(metadata.getLastAccessedUtcEpochSeconds(), actualMetadata.getLastAccessedUtcEpochSeconds());
Assertions.assertEquals(metadata.getCreatedUtcEpochSeconds(), actualMetadata.getCreatedUtcEpochSeconds());
//created time is not always set even on Unix
Assertions.assertEquals(metadata.getHidden(), actualMetadata.getHidden());
}

Expand All @@ -89,7 +89,7 @@ void testSetMetadataShouldNotSetPermissionsOwnerGroupButSetTimestampsWhenCalledW
Assertions.assertNotEquals(metadata.getPosixPermissions(), actualMetadata.getPosixPermissions());
Assertions.assertEquals(metadata.getLastModifiedUtcEpochSeconds(), actualMetadata.getLastModifiedUtcEpochSeconds());
Assertions.assertEquals(metadata.getLastAccessedUtcEpochSeconds(), actualMetadata.getLastAccessedUtcEpochSeconds());
Assertions.assertEquals(metadata.getCreatedUtcEpochSeconds(), actualMetadata.getCreatedUtcEpochSeconds());
//created time is not always set even on Unix
Assertions.assertEquals(metadata.getHidden(), actualMetadata.getHidden());
}

Expand Down Expand Up @@ -126,7 +126,7 @@ void testSetInitialPermissionsShouldSetPermissionsToAllowAllWhenCalledWithExisti
Assertions.assertEquals(expectedPath, actualMetadata.getAbsolutePath());
Assertions.assertEquals(FULL_ACCESS, actualMetadata.getPosixPermissions());
Assertions.assertNotEquals(metadata.getLastModifiedUtcEpochSeconds(), actualMetadata.getLastModifiedUtcEpochSeconds());
Assertions.assertNotEquals(metadata.getCreatedUtcEpochSeconds(), actualMetadata.getCreatedUtcEpochSeconds());
//created time is not always set even on Unix
}

@Test
Expand All @@ -151,7 +151,7 @@ void testSetPermissionsShouldSetPermissionsOnlyWhenCalledWithExistingFile() thro
Assertions.assertEquals(expectedPath, actualMetadata.getAbsolutePath());
Assertions.assertEquals(metadata.getPosixPermissions(), actualMetadata.getPosixPermissions());
Assertions.assertNotEquals(metadata.getLastModifiedUtcEpochSeconds(), actualMetadata.getLastModifiedUtcEpochSeconds());
Assertions.assertNotEquals(metadata.getCreatedUtcEpochSeconds(), actualMetadata.getCreatedUtcEpochSeconds());
//created time is not always set even on Unix
}

@Test
Expand Down Expand Up @@ -217,7 +217,7 @@ void testSetTimestampsShouldSetTimestampsWhenCalledWithExistingFile() throws IOE
Assertions.assertNotEquals(metadata.getCreatedUtcEpochSeconds(), original.getCreatedUtcEpochSeconds());
Assertions.assertEquals(metadata.getLastModifiedUtcEpochSeconds(), actualMetadata.getLastModifiedUtcEpochSeconds());
Assertions.assertEquals(metadata.getLastAccessedUtcEpochSeconds(), actualMetadata.getLastAccessedUtcEpochSeconds());
Assertions.assertEquals(metadata.getCreatedUtcEpochSeconds(), actualMetadata.getCreatedUtcEpochSeconds());
//created time is not always set even on Unix
}

@SuppressWarnings("DataFlowIssue")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.io.input.BoundedInputStream;
import org.apache.commons.io.input.CloseShieldInputStream;
import org.apache.commons.io.input.CountingInputStream;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -242,7 +242,7 @@ public void verifyHashes() throws IOException, ArchiveIntegrityException {
*/
public InputStream openStreamForSequentialAccess() throws IOException {
final var fileInputStream = new MergingFileInputStream(this.getAllFiles());
return new CountingInputStream(fileInputStream);
return BoundedInputStream.builder().setInputStream(fileInputStream).get();
}

/**
Expand Down Expand Up @@ -424,7 +424,7 @@ private boolean isArchiveHashValid(
digestCalculatorStream.close();
final var actualDigestValue = digestCalculatorStream.getDigestValue();
if (!Objects.equals(actualDigestValue, entry.getArchivedHash())) {
log.error("Hash mismatch for " + path);
log.error("Hash mismatch for {}", path);
return false;
} else {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import lombok.NonNull;
import org.apache.commons.io.IOUtils;
import org.apache.commons.io.input.CountingInputStream;
import org.apache.commons.io.input.BoundedInputStream;

import java.io.IOException;
import java.io.InputStream;
Expand All @@ -11,7 +11,7 @@
* Allows us to read a fixed range of bytes from an input stream and act as if the end of the stream
* was reached when the range is exhausted.
*/
public class FixedRangeInputStream extends CountingInputStream {
public class FixedRangeInputStream extends BoundedInputStream {

private final long endExclusive;

Expand All @@ -23,6 +23,7 @@ public class FixedRangeInputStream extends CountingInputStream {
* @param length the length of the range
* @throws IOException if the source stream cannot be read
*/
@SuppressWarnings("deprecation")
public FixedRangeInputStream(
@NonNull final InputStream source, final long startInclusive, final long length)
throws IOException {
Expand All @@ -39,7 +40,7 @@ public FixedRangeInputStream(

@Override
public int read() throws IOException {
if (getByteCount() >= endExclusive) {
if (getCount() >= endExclusive) {
return IOUtils.EOF;
}
return super.read();
Expand All @@ -52,10 +53,10 @@ public int read(final byte[] bts) throws IOException {

@Override
public int read(final byte[] bts, final int off, final int len) throws IOException {
if (getByteCount() >= endExclusive) {
if (getCount() >= endExclusive) {
return IOUtils.EOF;
}
final var allowedLength = endExclusive - getByteCount();
final var allowedLength = endExclusive - getCount();
return super.read(bts, off, (int) Math.min(len, allowedLength));
}
}

0 comments on commit f853b08

Please sign in to comment.