Skaffold system check (#158)

* Checks to see if skaffold is on system path

* "Squashed code for testing"

* skaffold new tests

* refactoring of system check tests

* refactoring of system check tests

* modified Test files to make them executable. mocked skaffold file to check existence

* fixed tests to include path

* removed notifications code

* refactored getSystemPath()

common-test-lib/src/main/kotlin/com/google/container/tools/test/ContainerToolsRule.kt

@@ -132,7 +132,6 @@ class ContainerToolsRule(private val testInstance: Any) : TestRule {
                     FileUtil.writeToFile(file, it.contents)
                 }
             }
-
             filesToDelete.add(file)
             member.setter.call(testInstance, file)
         }

skaffold/src/main/kotlin/com/google/container/tools/skaffold/SkaffoldExecutorService.kt

@@ -45,6 +45,15 @@ abstract class SkaffoldExecutorService {
     /** Path for Skaffold executable, any form supported by [ProcessBuilder] */
     protected abstract var skaffoldExecutablePath: Path
 
+    fun getSystemPath(): String = System.getenv("PATH")
+
+    fun isSkaffoldAvailable(): Boolean = getSystemPath().split(File.pathSeparator)
+            .asSequence()
+            .map { it + File.separator + "skaffold" } // convert each to a possible path to the executable
+            .any {
+                File(it).exists() && File(it).canExecute()
+            }
+
     /**
      * Creates Skaffold command line from the given settings and returns resulting launched
      * Skaffold system process.

skaffold/src/main/kotlin/com/google/container/tools/skaffold/run/SkaffoldCommandLineState.kt

@@ -46,6 +46,7 @@ class SkaffoldCommandLineState(
     val executionMode: SkaffoldExecutorSettings.ExecutionMode
 ) : CommandLineState(environment) {
     public override fun startProcess(): ProcessHandler {
+
         val runConfiguration: RunConfiguration? =
             environment.runnerAndConfigurationSettings?.configuration
         val projectBaseDir: VirtualFile? = environment.project.guessProjectDir()
@@ -60,6 +61,10 @@ class SkaffoldCommandLineState(
             throw ExecutionException(message("skaffold.no.file.selected.error"))
         }
 
+        if (!SkaffoldExecutorService.instance.isSkaffoldAvailable()) {
+            throw ExecutionException(message("skaffold.not.on.system.error"))
+        }
+
         val configFile: VirtualFile? = LocalFileSystem.getInstance()
             .findFileByPath(runConfiguration.skaffoldConfigurationFilePath!!)
         // use project dir relative location for cleaner command line representation
@@ -70,7 +75,6 @@ class SkaffoldCommandLineState(
         // custom settings for single deployment (run) mode
         val singleRunConfiguration: SkaffoldSingleRunConfiguration? =
             if (runConfiguration is SkaffoldSingleRunConfiguration) runConfiguration else null
-
         val skaffoldProcess = SkaffoldExecutorService.instance.executeSkaffold(
             SkaffoldExecutorSettings(
                 executionMode,
@@ -82,7 +86,6 @@ class SkaffoldCommandLineState(
                 defaultImageRepo = runConfiguration.imageRepositoryOverride
             )
         )
-
         return KillableProcessHandler(skaffoldProcess.process, skaffoldProcess.commandLine)
     }
 }

skaffold/src/main/kotlin/com/google/container/tools/skaffold/run/SkaffoldRunConfigurations.kt

@@ -16,14 +16,17 @@
 
 package com.google.container.tools.skaffold.run
 
+import com.google.container.tools.skaffold.SkaffoldExecutorService
 import com.google.container.tools.skaffold.SkaffoldExecutorSettings
+import com.google.container.tools.skaffold.message
 import com.google.container.tools.skaffold.run.ui.SkaffoldDevSettingsEditor
 import com.google.container.tools.skaffold.run.ui.SkaffoldSingleRunSettingsEditor
 import com.intellij.execution.Executor
 import com.intellij.execution.configurations.ConfigurationFactory
 import com.intellij.execution.configurations.RunConfiguration
 import com.intellij.execution.configurations.RunConfigurationBase
 import com.intellij.execution.configurations.RunProfileState
+import com.intellij.execution.configurations.RuntimeConfigurationWarning
 import com.intellij.execution.runners.ExecutionEnvironment
 import com.intellij.openapi.options.SettingsEditor
 import com.intellij.openapi.project.Project
@@ -101,4 +104,10 @@ abstract class AbstractSkaffoldRunConfiguration(
 
         XmlSerializer.serializeInto(this, element)
     }
+
+    override fun checkConfiguration() {
+        if (!SkaffoldExecutorService.instance.isSkaffoldAvailable()) {
+            throw RuntimeConfigurationWarning(message("skaffold.not.on.system.error"))
+        }
+    }
 }

skaffold/src/main/resources/messages/SkaffoldBundle.properties

@@ -42,3 +42,5 @@ skaffold.override.image.repo.tooltip=<html>The default image repository allows y
 skaffold.no.file.selected.error=Skaffold configuration file is not selected.
 skaffold.invalid.file.error=Skaffold configuration file doesn't exist or invalid.
 skaffold.corrupted.run.settings=Your Skaffold run configuration is corrupted. Please re-create it to fix.
+skaffold.not.on.system.title =Skaffold Error
+skaffold.not.on.system.error=Could not detect Skaffold installation. Please install Skaffold. See required dependencies <a href="https://github.com/GoogleContainerTools/google-container-tools-intellij#prerequisites-and-required-dependencies">here</a>

skaffold/src/test/kotlin/com/google/container/tools/skaffold/DefaultSkaffoldExecutorServiceTest.kt

@@ -18,6 +18,7 @@ package com.google.container.tools.skaffold
 
 import com.google.common.truth.Truth.assertThat
 import com.google.container.tools.test.ContainerToolsRule
+import com.google.container.tools.test.TestFile
 import io.mockk.every
 import io.mockk.impl.annotations.MockK
 import io.mockk.spyk
@@ -37,6 +38,12 @@ class DefaultSkaffoldExecutorServiceTest {
     @MockK
     private lateinit var mockProcess: Process
 
+    @TestFile(name = "skaffold", contents = "Some contents")
+    private lateinit var testSkaffoldFile: File
+
+    @TestFile(name = "notSkaffold", contents = "Some contents")
+    private lateinit var testNotSkaffoldFile: File
+
     @Before
     fun setUp() {
         defaultSkaffoldExecutorService = spyk(DefaultSkaffoldExecutorService())
@@ -226,4 +233,25 @@ class DefaultSkaffoldExecutorServiceTest {
 
         assertThat(result.commandLine).isEqualTo("skaffold dev --filename test.yaml")
     }
+
+    @Test
+    fun `isSkaffoldAvailable returns true when skaffold is available`() {
+        testSkaffoldFile.setExecutable(true)
+        every { defaultSkaffoldExecutorService.getSystemPath() } answers { testSkaffoldFile.parent }
+        assertThat(defaultSkaffoldExecutorService.isSkaffoldAvailable()).isTrue()
+    }
+
+    @Test
+    fun `isSkaffoldAvailable returns false when skaffold is not available`() {
+        every { defaultSkaffoldExecutorService.getSystemPath() } answers { "" }
+        assertThat(defaultSkaffoldExecutorService.isSkaffoldAvailable()).isFalse()
+    }
+
+    @Test
+    fun `isSkaffoldAvailable returns false when skaffold is not available in valid system paths`() {
+        every { defaultSkaffoldExecutorService.getSystemPath() } answers {
+            testNotSkaffoldFile.parent
+        }
+        assertThat(defaultSkaffoldExecutorService.isSkaffoldAvailable()).isFalse()
+    }
 }

skaffold/src/test/kotlin/com/google/container/tools/skaffold/run/SkaffoldCommandLineStateTest.kt

@@ -129,6 +129,8 @@ class SkaffoldCommandLineStateTest {
             projectBaseDir + "/skaffold.yaml"
         }
 
+        every { SkaffoldExecutorService.instance.isSkaffoldAvailable() } answers { true }
+
         skaffoldCommandLineState = SkaffoldCommandLineState(
             mockExecutionEnvironment,
             SkaffoldExecutorSettings.ExecutionMode.DEV
@@ -141,4 +143,32 @@ class SkaffoldCommandLineStateTest {
             )
         )
     }
+
+    @Test
+    fun `A run error is thrown if skaffold is not in the system PATH`() {
+        every { SkaffoldExecutorService.instance.isSkaffoldAvailable() } answers { false }
+
+        skaffoldCommandLineState = SkaffoldCommandLineState(
+            mockExecutionEnvironment,
+            SkaffoldExecutorSettings.ExecutionMode.DEV
+        )
+
+        expectThrows(
+            ExecutionException::class,
+            ThrowableRunnable { skaffoldCommandLineState.startProcess() })
+    }
+
+    @Test
+    fun `A run error is not thrown if skaffold is in the system PATH`() {
+        every { SkaffoldExecutorService.instance.isSkaffoldAvailable() } answers { true }
+
+        skaffoldCommandLineState = SkaffoldCommandLineState(
+            mockExecutionEnvironment,
+            SkaffoldExecutorSettings.ExecutionMode.DEV
+        )
+
+        skaffoldCommandLineState.startProcess()
+
+        assertThat(skaffoldSettingsCapturingSlot.captured.workingDirectory).isNotNull()
+    }
 }