fix: Don't print exception when copying across filesystems fails

[symwell]

Before this change, when moveTo copied a file across filesystems it triggered and caught an AtomicMoveNotSupportedException exception, printed it out, and then tried to copy the file in other ways which eventually succeeded. This had the side effect of printing the exception even though moveTo succeeded.

After this change, moveTo doesn't print any exceptions if it succeeds.

• Added two tests that verify the exceptions were and still are caught correctly. They are the tests testWriteFileAcrossFilesystems and testCantOverwriteTargetFile. Didn't add a third test for the third FileMover for Files.copy because it seems to be covered by testUnwriteableOutputFile.
• Added an integration test agent/test/filemover that verifies the Invalid cross-device link error message from the AtomicMoveNotSupportedException isn't printed.

Before the change in Recording.java, the new integration test failed.

~/src/appmap-java$ ./gradlew build; rm agent/test/filemover/*class; bin/test; (cd agent; java -javaagent:build/libs/appmap-1.15.5.jar -cp test/filemover:build/classes/java/test -Dappmap.record.private=true FileMover)
16 actionable tasks: 3 executed, 13 up-to-date
1..1
not ok 1 moveTo
# (from function `refute_output' in file test/filemover/../../build/bats/bats-assert/src/refute_output.bash, line 189,
#  in test file test/filemover/filemover.bats, line 27)
#   `refute_output -p "Invalid cross-device link"' failed
Test command ./test/filemover/test failed
java.nio.file.AtomicMoveNotSupportedException: /tmp/recordertest_file -> /dev/shm/recordertest_file: Invalid cross-device link
    at java.base/sun.nio.fs.UnixCopyFile.move(UnixCopyFile.java:415)
    at java.base/sun.nio.fs.UnixFileSystemProvider.move(UnixFileSystemProvider.java:267)
    at java.base/java.nio.file.Files.move(Files.java:1422)
    at com.appland.appmap.record.Recording.lambda$moveTo$1(Recording.java:53)
    at com.appland.appmap.record.Recording.lambda$moveTo$0(Recording.java:45)
    at com.appland.appmap.record.Recording.moveTo(Recording.java:65)
    at FileMover.main(FileMover.java:28)

After the change in Recording.java, the new integration test passes.

~/src/appmap-java$ ./gradlew build; rm agent/test/filemover/*class; bin/test; (cd agent; java -javaagent:build/libs/appmap-1.15.5.jar -cp test/filemover:build/classes/java/test -Dappmap.record.private=true FileMover)
16 actionable tasks: 6 executed, 10 up-to-date
1..1
ok 1 moveTo
test@work[2023-02-06_12:00:30]:~/src/appmap-java$

Fixes https://github.com/getappmap/appmap-java/issues/162

[apotterri]

The commit tagged refactor should really have been a fixup! commit. Please be sure to combine it with the previous one before merging.

[apotterri]

I know it wasn't part of your changes, but can you update this test, too, please:

  @Test
  public void testUnwriteableOutputFile() throws IOException {
    Recorder recorder = recordEvents();
    final Recording recording = recorder.stop();
    Exception exception = null;
    try {
      recording.moveTo("/no-such-directory");
    } catch (RuntimeException e) {
      exception = e;
    }
    assertNotNull(exception);
    assertTrue(exception.toString().indexOf("java.lang.RuntimeException: ") == 0);
  }

recording.moveTo("/no-such-directory") succeeds if the user has permission to write /. This can happen if you're running the tests in a docker container, e.g. on macOS to make sure they all pass.

Changing it to recording.moveTo("/no-such-directory/.") will cause it to fail as expected.

[symwell]

Changing it to recording.moveTo("/no-such-directory/.") will cause it to fail as expected.

Good catch! Changed.

[appland-release]

:tada: This PR is included in version 1.15.6 :tada:

The release is available on:

• v1.15.6
• GitHub release

Your semantic-release bot :package::rocket: