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

Improve error message on IOS #968

Merged
merged 7 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion release_notes.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## next (unreleased)
- [#952](https://github.com/Flank/flank/pull/952) Fix version printing on Flank release
- [#937](https://github.com/Flank/flank/pull/968) Improve error message on IOS when test or xctestrun-file not found ([sloox](https://github.com/Sloox))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- [#937](https://github.com/Flank/flank/pull/968) Improve error message on IOS when test or xctestrun-file not found ([sloox](https://github.com/Sloox))
- [#937](https://github.com/Flank/flank/pull/968) Improve error message on iOS when test or xctestrun-file not found ([sloox](https://github.com/Sloox))

- [#952](https://github.com/Flank/flank/pull/952) Fix version printing on Flank release ([sloox](https://github.com/Sloox))
- [#950](https://github.com/Flank/flank/pull/950) Fix crash when --legacy-junit-result set. ([adamfilipow92](https://github.com/adamfilipow92))
- [#948](https://github.com/Flank/flank/pull/948) Increment retry tries and change sync tag for jfrogSync. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#946](https://github.com/Flank/flank/pull/946) Added tests for flank scripts. ([piotradamczyk5](https://github.com/piotradamczyk5))
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 @@ -62,7 +62,7 @@ IosArgs
private fun IosArgs.calculateShardChunks() = if (disableSharding)
listOf(emptyList()) else
ArgsHelper.calculateShards(
filteredTests = filterTests(findTestNames(xctestrunFile), testTargets)
filteredTests = filterTests(findTestNames(xctestrunFile.orEmpty()), testTargets)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xctestrunFile is not nullable so .orEmpty() is redundant here.

.distinct()
.map { FlankTestMethod(it, ignored = false) },
args = this
Expand Down
5 changes: 5 additions & 0 deletions test_runner/src/main/kotlin/ftl/args/ValidateIosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import ftl.util.IncompatibleTestDimensionError
fun IosArgs.validate() {
assertXcodeSupported()
assertDevicesSupported()
assertTestTypes()
assertMaxTestShards()
}
private fun IosArgs.assertMaxTestShards() { this.maxTestShards
Expand All @@ -17,6 +18,10 @@ private fun IosArgs.assertMaxTestShards() { this.maxTestShards
"max-test-shards must be >= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, or -1. But current is $maxTestShards"
)
}
private fun IosArgs.assertTestTypes() {
if (xctestrunFile.isNullOrBlank() or xctestrunZip.isNullOrBlank())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isBlank() should be enough in both cases

Copy link
Contributor

@adamfilipow92 adamfilipow92 Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file, we have two logic operator convention now. Look:

private fun IosArgs.assertMaxTestShards() { this.maxTestShards
    if (
        maxTestShards !in IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE &&
        maxTestShards != -1
    ) throw FlankConfigurationError(
        "max-test-shards must be >= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, or -1. But current is $maxTestShards"
    )
}
private fun IosArgs.assertTestTypes() {
    if (xctestrunFile.isNullOrBlank() or xctestrunZip.isNullOrBlank())
        throw FlankConfigurationError("Both of following options must be specified [test, xctestrun-file].")
}
private fun IosArgs.assertXcodeSupported() = when {
    xcodeVersion == null -> Unit
    IosCatalog.supportedXcode(xcodeVersion, project) -> Unit
    else -> throw IncompatibleTestDimensionError(("Xcode $xcodeVersion is not a supported Xcode version"))
}

Maybe we should decide and keep only one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 @adamfilipow92 we can consider it in another issue. This PR name is Improve error message on IOS not Android

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, my mistake. I update my comment to iOS version.

Copy link
Contributor Author

@Sloox Sloox Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall i create a ticket for this @jan-gogo @adamfilipowicz92 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sloox hehe, ofc not if it is only about style convention ;). It is minor for me, you can leave it as-is if you find it readable or fix if you want.

throw FlankConfigurationError("Both of following options must be specified [test, xctestrun-file].")
}
private fun IosArgs.assertXcodeSupported() = when {
xcodeVersion == null -> Unit
IosCatalog.supportedXcode(xcodeVersion, project) -> Unit
Expand Down
47 changes: 46 additions & 1 deletion test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,51 @@ IosArgs
val args = IosArgs.load(yaml)
assertEquals(IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last, args.maxTestShards)
}

@Test(expected = FlankConfigurationError::class)
fun `verify appropriate error message when test and xctestrun not set`() {
val yaml = """
gcloud:
flank:
max-test-shards: -1
""".trimIndent()
IosArgs.load(yaml)
}

@Test(expected = FlankConfigurationError::class)
fun `verify appropriate error message when xctestrun not set`() {
val yaml = """
gcloud:
test: $testPath
flank:
max-test-shards: -1
""".trimIndent()
IosArgs.load(yaml)
}

@Test(expected = FlankConfigurationError::class)
fun `verify appropriate error message when test not set`() {
val yaml = """
gcloud:
xctestrun-file: $testPath
flank:
max-test-shards: -1
""".trimIndent()
IosArgs.load(yaml)
}

@Test
fun `verify no error message when test and xctestrun-file set`() {
val yaml = """
gcloud:
test: $testPath
xctestrun-file: $testPath
flank:
max-test-shards: -1
""".trimIndent()
IosArgs.load(yaml)
}
}

private fun IosArgs.Companion.load(yamlData: String, cli: IosRunCommand? = null): IosArgs = load(StringReader(yamlData), cli)
private fun IosArgs.Companion.load(yamlData: String, cli: IosRunCommand? = null): IosArgs =
load(StringReader(yamlData), cli)