[CORDA-2275] Fix DriverTests for windows (#4325)

* Make enforceSingleNodeIsRunning internal for test purposes.

* Test that the file is deleted or the file lock is released to be sure that the node has stopped.

* Additional test for enforceSingleNodeIsRunnin

node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt

@@ -253,7 +253,7 @@ open class NodeStartup : NodeStartupLogging {
         }
     }
 
-    private fun enforceSingleNodeIsRunning(baseDirectory: Path) {
+    internal fun enforceSingleNodeIsRunning(baseDirectory: Path) {
         // Write out our process ID (which may or may not resemble a UNIX process id - to us it's just a string) to a
         // file that we'll do our best to delete on exit. But if we don't, it'll be overwritten next time. If it already
         // exists, we try to take the file lock first before replacing it and if that fails it means we're being started

node/src/test/kotlin/net/corda/node/internal/NodeStartupCliTest.kt

@@ -0,0 +1,52 @@
+package net.corda.node.internal
+
+import net.corda.cliutils.CommonCliConstants
+import net.corda.core.internal.div
+import net.corda.nodeapi.internal.config.UnknownConfigKeysPolicy
+import org.assertj.core.api.Assertions
+import org.junit.BeforeClass
+import org.junit.Test
+import org.slf4j.event.Level
+import picocli.CommandLine
+import java.nio.file.Path
+import java.nio.file.Paths
+
+class NodeStartupCliTest {
+    private val startup = NodeStartupCli()
+
+    companion object {
+        private lateinit var workingDirectory: Path
+
+        @BeforeClass
+        @JvmStatic
+        fun initDirectories() {
+            workingDirectory = Paths.get(".").normalize().toAbsolutePath()
+        }
+    }
+
+    @Test
+    fun `no command line arguments`() {
+        CommandLine.populateCommand(startup)
+        Assertions.assertThat(startup.cmdLineOptions.baseDirectory).isEqualTo(workingDirectory)
+        Assertions.assertThat(startup.cmdLineOptions.configFile).isEqualTo(workingDirectory / "node.conf")
+        Assertions.assertThat(startup.verbose).isEqualTo(false)
+        Assertions.assertThat(startup.loggingLevel).isEqualTo(Level.INFO)
+        Assertions.assertThat(startup.cmdLineOptions.noLocalShell).isEqualTo(false)
+        Assertions.assertThat(startup.cmdLineOptions.sshdServer).isEqualTo(false)
+        Assertions.assertThat(startup.cmdLineOptions.justGenerateNodeInfo).isEqualTo(false)
+        Assertions.assertThat(startup.cmdLineOptions.justGenerateRpcSslCerts).isEqualTo(false)
+        Assertions.assertThat(startup.cmdLineOptions.unknownConfigKeysPolicy)
+                .isEqualTo(UnknownConfigKeysPolicy.FAIL)
+        Assertions.assertThat(startup.cmdLineOptions.devMode).isEqualTo(null)
+        Assertions.assertThat(startup.cmdLineOptions.clearNetworkMapCache).isEqualTo(false)
+        Assertions.assertThat(startup.cmdLineOptions.networkRootTrustStorePathParameter).isEqualTo(null)
+    }
+
+    @Test
+    fun `--base-directory`() {
+        CommandLine.populateCommand(startup, CommonCliConstants.BASE_DIR, (workingDirectory / "another-base-dir").toString())
+        Assertions.assertThat(startup.cmdLineOptions.baseDirectory).isEqualTo(workingDirectory / "another-base-dir")
+        Assertions.assertThat(startup.cmdLineOptions.configFile).isEqualTo(workingDirectory / "another-base-dir" / "node.conf")
+        Assertions.assertThat(startup.cmdLineOptions.networkRootTrustStorePathParameter).isEqualTo(null)
+    }
+}

node/src/test/kotlin/net/corda/node/internal/NodeStartupTest.kt

@@ -1,51 +1,30 @@
 package net.corda.node.internal
 
-import net.corda.cliutils.CommonCliConstants.BASE_DIR
-import net.corda.core.internal.div
-import net.corda.nodeapi.internal.config.UnknownConfigKeysPolicy
-import org.assertj.core.api.Assertions.assertThat
-import org.junit.BeforeClass
+import com.google.common.io.Files
 import org.junit.Test
-import org.slf4j.event.Level
-import picocli.CommandLine
-import java.nio.file.Path
-import java.nio.file.Paths
+import java.nio.channels.OverlappingFileLockException
+import java.util.concurrent.CountDownLatch
+import kotlin.concurrent.thread
+import kotlin.test.assertFailsWith
 
 class NodeStartupTest {
-    private val startup = NodeStartupCli()
+    @Test
+    fun `test that you cant start two nodes in the same directory`() {
+        val dir = Files.createTempDir().toPath()
 
-    companion object {
-        private lateinit var workingDirectory: Path
+        val latch = CountDownLatch(1)
 
-        @BeforeClass
-        @JvmStatic
-        fun initDirectories() {
-            workingDirectory = Paths.get(".").normalize().toAbsolutePath()
+        thread(start = true) {
+            val node = NodeStartup()
+            node.enforceSingleNodeIsRunning(dir)
+            latch.countDown()
         }
-    }
 
-    @Test
-    fun `no command line arguments`() {
-        CommandLine.populateCommand(startup)
-        assertThat(startup.cmdLineOptions.baseDirectory).isEqualTo(workingDirectory)
-        assertThat(startup.cmdLineOptions.configFile).isEqualTo(workingDirectory / "node.conf")
-        assertThat(startup.verbose).isEqualTo(false)
-        assertThat(startup.loggingLevel).isEqualTo(Level.INFO)
-        assertThat(startup.cmdLineOptions.noLocalShell).isEqualTo(false)
-        assertThat(startup.cmdLineOptions.sshdServer).isEqualTo(false)
-        assertThat(startup.cmdLineOptions.justGenerateNodeInfo).isEqualTo(false)
-        assertThat(startup.cmdLineOptions.justGenerateRpcSslCerts).isEqualTo(false)
-        assertThat(startup.cmdLineOptions.unknownConfigKeysPolicy).isEqualTo(UnknownConfigKeysPolicy.FAIL)
-        assertThat(startup.cmdLineOptions.devMode).isEqualTo(null)
-        assertThat(startup.cmdLineOptions.clearNetworkMapCache).isEqualTo(false)
-        assertThat(startup.cmdLineOptions.networkRootTrustStorePathParameter).isEqualTo(null)
-    }
+        // wait until the file has been created on the other thread
+        latch.await()
 
-    @Test
-    fun `--base-directory`() {
-        CommandLine.populateCommand(startup, BASE_DIR, (workingDirectory / "another-base-dir").toString())
-        assertThat(startup.cmdLineOptions.baseDirectory).isEqualTo(workingDirectory / "another-base-dir")
-        assertThat(startup.cmdLineOptions.configFile).isEqualTo(workingDirectory / "another-base-dir" / "node.conf")
-        assertThat(startup.cmdLineOptions.networkRootTrustStorePathParameter).isEqualTo(null)
+        // Check that I can't start up another node in the same directory
+        val anotherNode = NodeStartup()
+        assertFailsWith<OverlappingFileLockException> { anotherNode.enforceSingleNodeIsRunning(dir) }
     }
-}
+}

testing/node-driver/src/integration-test/kotlin/net/corda/testing/driver/DriverTests.kt

@@ -2,13 +2,10 @@ package net.corda.testing.driver
 
 import net.corda.core.concurrent.CordaFuture
 import net.corda.core.identity.CordaX500Name
-import net.corda.core.internal.CertRole
+import net.corda.core.internal.*
 import net.corda.core.internal.concurrent.fork
 import net.corda.core.internal.concurrent.openFuture
 import net.corda.core.internal.concurrent.transpose
-import net.corda.core.internal.div
-import net.corda.core.internal.list
-import net.corda.core.internal.readLines
 import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.core.utilities.getOrThrow
 import net.corda.node.internal.NodeStartup
@@ -25,6 +22,7 @@ import net.corda.testing.node.internal.internalDriver
 import org.assertj.core.api.Assertions.*
 import org.json.simple.JSONObject
 import org.junit.Test
+import java.io.RandomAccessFile
 import java.util.*
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.Executors
@@ -119,15 +117,20 @@ class DriverTests {
     fun `started node, which is not waited for in the driver, is shutdown when the driver exits`() {
         // First check that the process-id file is created by the node on startup, so that we can be sure our check that
         // it's deleted on shutdown isn't a false-positive.
-        driver {
+        val baseDirectory = driver {
             val baseDirectory = defaultNotaryNode.getOrThrow().baseDirectory
             assertThat(baseDirectory / "process-id").exists()
+            baseDirectory
         }
 
-        val baseDirectory = internalDriver(notarySpecs = listOf(NotarySpec(DUMMY_NOTARY_NAME))) {
-            baseDirectory(DUMMY_NOTARY_NAME)
+        if ((baseDirectory / "process-id").exists()) {
+            // The addShutdownHook call doesn't reliably get called on Windows (even on graceful node shutdown), so at least check
+            // that the lock has been released, to make sure the node has been killed
+            val pidFile = (baseDirectory / "process-id").toFile()
+            val pidFileRw = RandomAccessFile(pidFile, "rw")
+            pidFileRw.channel.tryLock()
+            pidFileRw.close()
         }
-        assertThat(baseDirectory / "process-id").doesNotExist()
     }
 
     @Test