@gcliff I meant what is inside the folders. I am assuming the mp3 files created during the test.
I will disable the tests on Windows using @achilep proposal for now. The tests will still run in all our pipelines which ensures the behavior of the code will not change. In the meantime I will try to replicate your environment and the errors you are seeing. So that I can investigate more easily and look for a fix
This will be hard
So @gcliff my research tells me you run Windows 7, correct?
I was able to run the tests inside a Microsoft provided docker image with JDK8 https://hub.docker.com/_/microsoft-java-jdk but I assume that was naive since that is most likely very different from actual Windows since its meant for deploying things to Azure.
for the curious this is what you do
docker pull mcr.microsoft.com/java/maven:8-zulu-debian9
docker run -it --name windows -v ~/.m2:/root/.m2 -v ~/code/openmrs/openmrs-core:/code --workdir /code mcr.microsoft.com/java/maven:8-zulu-debian9 sh
mvn clean test -Dtest=BinaryStreamHandlerTest,MediaHandlerTest
I will most likely need to setup a VM. Not sure yet if I can get a Windows 7.
Just to point this out, you’ve actually pulled a Microsoft-provided Debian image, which really doesn’t help with the Windows-specific file handling issue. The minimal Windows image seems to be called 8-zulu-windowsservercore…
They’re generated here in the BaseContextSensitiveTest class which means that we generate probably 200ish directories per Maven build. They should be deleted on exit, but apparently some aren’t. Although I don’t get the exception you’re reporting on my machine (I use a Mac), I do see a number of those appdir-for-unit-tests- directories piled up in my temporary files location. When I clean the temp directory and re-run the tests, I end up with 11 hanging appdir-for-unit-tests- directories.
Thanks at @ibacher, if you remove the @DisabledOnOs(WINDOWS) flag on for example the MediaHandlerTest does it fail telling you the Jupiter Extension could not delete the temp dir because some other process was holding on to things in there (like in @gcliff logs)?
Can you @ibacher and @gcliff please run the MediaHandlerTest and BinaryStreamHandlerTest (remove the @DisabledOn) and let me know if that changes anything?
As I said before, I’m running a Mac, so I can’t reproduce the issue locally . I was simply pointing out that we do seem to be leaving other test-related detritus around.
That is yet another story. Did not know we do that. The issue we are seeing on Windows is that JUnit 5 @TempDir cannot remove the temp dir since some things in our code are still holding on to the files created in these temp dirs. These JUnit temp dirs are not the ones you listed.
@gcliff can you please try my suggestion since I and Ian cannot reproduce your error. You will need to pull the latest changes and remove the @DisabledOn on these 2 failing tests.
Anyone out there developing on Windows that can try the above? I tried all kinds of things to install a Windows VM/container on my machine but failed so far. I cannot reproduce the error @gcliff is seeing on Windows and thus not test if my change fixed it. Thanks a lot!
Bright side is we can keep the tests disabled on master on Windows so devs on Windows can work. Meanwhile we can work on a branch like https://github.com/openmrs/openmrs-core/pull/3380 where we enable them, make changes and they will be tested on Windows
That is obviously not a fix because it means that after doing handler.getObs(obs1, "RAW_VIEW").getComplexData().getData() the FileInputStream is already closed an cannot be read.
Is it something super obvious that I am not seeing because I am tired
I am unsure about this API. The data is of type Object how does anyone know that it is a stream or when it is a stream? What type of stream? And that its already open? And that one should make sure to close it.
What are our options? Any ideas. I am not sure what to do with these tests or the behaviour. I do think the JUnit 5 implementation of @TempDir seems more strict, which is good as it can uncover such things.