Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix execution hangs #657

Merged
merged 7 commits into from
Mar 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions release_notes.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
## next (unreleased)

- [#654](https://github.com/Flank/flank/pull/654) Fix test filters when using both notPackage and notClass. ([jan-gogo](https://github.com/jan-gogo))
- [#648](https://github.com/Flank/flank/pull/648) Include @Ignore JUnit tests in JUnit XML. ([pawelpasterz](https://github.com/pawelpasterz))
- [#657](https://github.com/Flank/flank/pull/657) Fix execution hangs. ([pawelpasterz](https://github.com/pawelpasterz))
- [#654](https://github.com/Flank/flank/pull/654) Fix test filters when using both notPackage and notClass. ([jan-gogo](https://github.com/jan-gogo))
- [#648](https://github.com/Flank/flank/pull/648) Include @Ignore JUnit tests in JUnit XML. ([pawelpasterz](https://github.com/pawelpasterz))
- [#646](https://github.com/Flank/flank/pull/646) Adopt kotlin-logging as a logging framework. ([jan-gogo](https://github.com/jan-gogo))
- [#644](https://github.com/Flank/flank/pull/644) Use high performance options by default. Video, login, and perf metrics are now disabled by default. ([pawelpasterz](https://github.com/pawelpasterz))
- [#643](https://github.com/Flank/flank/pull/643) Add --dry option to android run & ios run. ([jan-gogo](https://github.com/jan-gogo))
Expand Down
10 changes: 7 additions & 3 deletions test_runner/src/main/kotlin/ftl/Main.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import ftl.cli.firebase.RefreshCommand
import ftl.cli.firebase.test.AndroidCommand
import ftl.cli.firebase.test.IosCommand
import ftl.log.setDebugLogging
import ftl.util.Utils.readRevision
import ftl.util.Utils.readVersion
import ftl.util.readRevision
import ftl.util.readVersion
import ftl.util.jvmHangingSafe
import picocli.CommandLine

@CommandLine.Command(
Expand Down Expand Up @@ -45,7 +46,10 @@ class Main : Runnable {
companion object {
@JvmStatic
fun main(args: Array<String>) {
CommandLine(Main()).execute(*args)
// BugSnag opens a non-daemon thread which will keep the JVM process alive.
// Flank must invoke exitProcess to exit cleanly.
// https://github.com/bugsnag/bugsnag-java/issues/151
jvmHangingSafe { CommandLine(Main()).execute(*args) }
}
}
}
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import ftl.args.yml.YamlDeprecated
import ftl.cli.firebase.test.android.AndroidRunCommand
import ftl.config.Device
import ftl.config.FtlConstants
import ftl.util.Utils.fatalError
import ftl.util.fatalError
import java.nio.file.Files
import java.nio.file.Path

Expand Down
6 changes: 3 additions & 3 deletions test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import ftl.filter.TestFilter
import ftl.filter.TestFilters
import ftl.gc.GcStorage
import ftl.util.FlankTestMethod
import ftl.util.Utils
import ftl.util.fatalError
import kotlinx.coroutines.runBlocking

object AndroidTestShard {
Expand All @@ -32,7 +32,7 @@ object AndroidTestShard {
val shouldThrowErrorIfMissingTests = allTestMethods.isEmpty() && !args.disableSharding
when {
shouldIgnoreMissingTests -> return mutableListOf()
shouldThrowErrorIfMissingTests -> throw IllegalStateException(Utils.fatalError("Test APK has no tests"))
shouldThrowErrorIfMissingTests -> throw IllegalStateException(fatalError("Test APK has no tests"))
}
val testFilter = TestFilters.fromTestTargets(args.testTargets)
return allTestMethods filterWith testFilter
Expand All @@ -44,7 +44,7 @@ object AndroidTestShard {
.map { FlankTestMethod("class ${it.testName}", it.isIgnored) }
.toList()
.also {
require(FtlConstants.useMock || it.isNotEmpty()) { Utils.fatalError("All tests filtered out") }
require(FtlConstants.useMock || it.isNotEmpty()) { fatalError("All tests filtered out") }
}
}

Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/args/ArgsFileVisitor.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package ftl.args

import ftl.util.Utils.fatalError
import ftl.util.fatalError
import java.io.IOException
import java.lang.RuntimeException
import java.nio.file.FileSystems
Expand Down
27 changes: 14 additions & 13 deletions test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import ftl.shard.Shard
import ftl.shard.StringShards
import ftl.shard.stringShards
import ftl.util.FlankTestMethod
import ftl.util.Utils
import ftl.util.assertNotEmpty
import ftl.util.fatalError
import java.io.File
import java.net.URI
import java.nio.file.Files
Expand All @@ -49,29 +50,29 @@ object ArgsHelper {

fun assertFileExists(file: String, name: String) {
if (!File(file).exists()) {
Utils.fatalError("'$file' $name doesn't exist")
fatalError("'$file' $name doesn't exist")
}
}

fun assertCommonProps(args: IArgs) {
Utils.assertNotEmpty(
assertNotEmpty(
args.project, "The project is not set. Define GOOGLE_CLOUD_PROJECT, set project in flank.yml\n" +
"or save service account credential to ${defaultCredentialPath}\n" +
" See https://github.com/GoogleCloudPlatform/google-cloud-java#specifying-a-project-id"
)

if (args.maxTestShards !in IArgs.AVAILABLE_SHARD_COUNT_RANGE && args.maxTestShards != -1)
Utils.fatalError("max-test-shards must be >= 1 and <= 50, or -1. But current is ${args.maxTestShards}")
fatalError("max-test-shards must be >= 1 and <= 50, or -1. But current is ${args.maxTestShards}")

if (args.shardTime <= 0 && args.shardTime != -1) Utils.fatalError("shard-time must be >= 1 or -1")
if (args.repeatTests < 1) Utils.fatalError("num-test-runs must be >= 1")
if (args.shardTime <= 0 && args.shardTime != -1) fatalError("shard-time must be >= 1 or -1")
if (args.repeatTests < 1) fatalError("num-test-runs must be >= 1")

if (args.smartFlankGcsPath.isNotEmpty()) {
if (!args.smartFlankGcsPath.startsWith(GCS_PREFIX)) {
Utils.fatalError("smart-flank-gcs-path must start with gs://")
fatalError("smart-flank-gcs-path must start with gs://")
}
if (args.smartFlankGcsPath.count { it == '/' } <= 2 || !args.smartFlankGcsPath.endsWith(".xml")) {
Utils.fatalError("smart-flank-gcs-path must be in the format gs://bucket/foo.xml")
fatalError("smart-flank-gcs-path must be in the format gs://bucket/foo.xml")
}
}
}
Expand All @@ -87,9 +88,9 @@ object ArgsHelper {

val filePaths = walkFileTree(file)
if (filePaths.size > 1) {
Utils.fatalError("'$file' ($filePath) matches multiple files: $filePaths")
fatalError("'$file' ($filePath) matches multiple files: $filePaths")
} else if (filePaths.isEmpty()) {
Utils.fatalError("'$file' not found ($filePath)")
fatalError("'$file' not found ($filePath)")
}

return filePaths.first().toAbsolutePath().normalize().toString()
Expand All @@ -107,7 +108,7 @@ object ArgsHelper {
val blob = GcStorage.storage.get(bucket, path)

if (blob == null) {
Utils.fatalError("The file at '$uri' does not exist")
fatalError("The file at '$uri' does not exist")
}
}

Expand All @@ -119,8 +120,8 @@ object ArgsHelper {
) {
val missingMethods = testTargets - validTestMethods

if (!skipValidation && missingMethods.isNotEmpty()) Utils.fatalError("$from is missing methods: $missingMethods.\nValid methods:\n$validTestMethods")
if (validTestMethods.isEmpty()) Utils.fatalError("$from has no tests")
if (!skipValidation && missingMethods.isNotEmpty()) fatalError("$from is missing methods: $missingMethods.\nValid methods:\n$validTestMethods")
if (validTestMethods.isEmpty()) fatalError("$from has no tests")
}

fun createJunitBucket(projectId: String, junitGcsPath: String) {
Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/args/IosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import ftl.config.FtlConstants
import ftl.ios.IosCatalog
import ftl.ios.Xctestrun
import ftl.util.FlankTestMethod
import ftl.util.Utils.fatalError
import ftl.util.fatalError
import java.nio.file.Files
import java.nio.file.Path

Expand Down
6 changes: 2 additions & 4 deletions test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory
import com.fasterxml.jackson.databind.node.MissingNode
import com.fasterxml.jackson.databind.node.ObjectNode
import ftl.args.ArgsHelper.yamlMapper
import ftl.args.yml.YamlDeprecated.replace
import ftl.util.Utils
import ftl.util.Utils.fatalError
import ftl.util.fatalError
import java.nio.file.Files
import java.nio.file.Path

Expand Down Expand Up @@ -140,7 +138,7 @@ object YamlDeprecated {

if (error) {
val platform = if (android) "android" else "ios"
Utils.fatalError("Invalid keys detected! Auto fix with: flank $platform doctor --fix")
fatalError("Invalid keys detected! Auto fix with: flank $platform doctor --fix")
}

return data
Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/config/Device.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package ftl.config

import ftl.config.FtlConstants.defaultLocale
import ftl.config.FtlConstants.defaultOrientation
import ftl.util.Utils.trimStartLine
import ftl.util.trimStartLine

data class Device(
val model: String,
Expand Down
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/config/FtlConstants.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import ftl.args.IArgs
import ftl.args.IosArgs
import ftl.gc.UserAuth
import ftl.http.HttpTimeoutIncrease
import ftl.util.Utils.fatalError
import ftl.util.Utils.readRevision
import ftl.util.fatalError
import ftl.util.readRevision
import java.io.IOException
import java.nio.file.Path
import java.nio.file.Paths
Expand Down
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/gc/GcAndroidTestMatrix.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import com.google.api.services.testing.model.TestSpecification
import com.google.api.services.testing.model.TestTargetsForShard
import com.google.api.services.testing.model.ToolResultsHistory
import ftl.args.AndroidArgs
import ftl.util.Utils.fatalError
import ftl.util.Utils.join
import ftl.util.fatalError
import ftl.util.join
import ftl.util.testTimeoutToSeconds

object GcAndroidTestMatrix {
Expand Down
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/gc/GcIosTestMatrix.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import ftl.args.IosArgs
import ftl.ios.Xctestrun
import ftl.ios.Xctestrun.toByteArray
import ftl.util.ShardCounter
import ftl.util.Utils.fatalError
import ftl.util.Utils.join
import ftl.util.fatalError
import ftl.util.join
import ftl.util.testTimeoutToSeconds

object GcIosTestMatrix {
Expand Down
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/gc/GcStorage.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import ftl.reports.xml.model.JUnitTestResult
import ftl.reports.xml.parseAllSuitesXml
import ftl.reports.xml.xmlToString
import ftl.util.ProgressBar
import ftl.util.Utils.fatalError
import ftl.util.Utils.join
import ftl.util.fatalError
import ftl.util.join
import java.io.File
import java.io.FileOutputStream
import java.net.URI
Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/gc/GcTestMatrix.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.google.api.services.testing.model.CancelTestMatrixResponse
import com.google.api.services.testing.model.TestMatrix
import ftl.args.IArgs
import ftl.http.executeWithRetry
import ftl.util.Utils.sleep
import ftl.util.sleep
import java.time.Duration.ofHours

object GcTestMatrix {
Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/ios/Parse.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package ftl.ios

import ftl.config.FtlConstants.isMacOS
import ftl.util.Bash
import ftl.util.Utils.copyBinaryResource
import ftl.util.copyBinaryResource
import java.io.File

object Parse {
Expand Down
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/reports/CostReport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import ftl.json.MatrixMap
import ftl.reports.util.IReport
import ftl.reports.xml.model.JUnitTestResult
import ftl.util.Billing
import ftl.util.Utils.println
import ftl.util.Utils.write
import ftl.util.println
import ftl.util.write
import java.io.StringWriter

/** Calculates cost based on the matrix map. Always run. */
Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/reports/HtmlErrorReport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import ftl.json.MatrixMap
import ftl.reports.util.IReport
import ftl.reports.xml.model.JUnitTestCase
import ftl.reports.xml.model.JUnitTestResult
import ftl.util.Utils.readTextResource
import ftl.util.readTextResource
import java.nio.file.Files
import java.nio.file.Paths

Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/reports/JUnitReport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import ftl.json.MatrixMap
import ftl.reports.util.IReport
import ftl.reports.xml.model.JUnitTestResult
import ftl.reports.xml.xmlToString
import ftl.util.Utils.write
import ftl.util.write

/** Calculates cost based on the matrix map. Always run. */
object JUnitReport : IReport {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import ftl.config.FtlConstants.indent
import ftl.json.MatrixMap
import ftl.reports.util.IReport
import ftl.reports.xml.model.JUnitTestResult
import ftl.util.Utils.println
import ftl.util.Utils.write
import ftl.util.println
import ftl.util.write
import java.io.StringWriter
import java.text.DecimalFormat

Expand Down
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/run/GenericTestRunner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import ftl.config.FtlConstants.useMock
import ftl.json.MatrixMap
import ftl.json.SavedMatrix
import ftl.util.StopWatch
import ftl.util.Utils
import ftl.util.uniqueObjectName
import java.io.File

object GenericTestRunner {
Expand All @@ -16,7 +16,7 @@ object GenericTestRunner {
val stopwatch = StopWatch().start()
TestRunner.assertMockUrl()

val runGcsPath = args.resultsDir ?: Utils.uniqueObjectName()
val runGcsPath = args.resultsDir ?: uniqueObjectName()

// Avoid spamming the results/ dir with temporary files from running the test suite.
if (useMock) {
Expand Down
6 changes: 3 additions & 3 deletions test_runner/src/main/kotlin/ftl/run/TestRunner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import ftl.util.MatrixState
import ftl.util.ObjPath
import ftl.util.StopWatch
import ftl.util.StopWatchMatrix
import ftl.util.Utils
import ftl.util.Utils.fatalError
import ftl.util.completed
import ftl.util.fatalError
import ftl.util.sleep
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
Expand Down Expand Up @@ -310,7 +310,7 @@ object TestRunner {

// GetTestMatrix is not designed to handle many requests per second.
// Sleep to avoid overloading the system.
Utils.sleep(5)
sleep(5)
refreshedMatrix = GcTestMatrix.refresh(matrixId, args)
}

Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/shard/Shard.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import ftl.args.IosArgs
import ftl.reports.xml.model.JUnitTestCase
import ftl.reports.xml.model.JUnitTestResult
import ftl.util.FlankTestMethod
import ftl.util.Utils.fatalError
import ftl.util.fatalError
import kotlin.math.ceil
import kotlin.math.min
import kotlin.math.roundToInt
Expand Down
1 change: 0 additions & 1 deletion test_runner/src/main/kotlin/ftl/util/MatrixUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package ftl.util

import ftl.args.IArgs
import ftl.json.MatrixMap
import ftl.util.Utils.join
import java.io.File
import java.util.concurrent.TimeUnit

Expand Down
Loading