#816 Omit `environment-variables` for robo test executions instead of throwing exception. #826

adamfilipow92 · 2020-05-29T15:34:20Z

Fixes #816

Checklist

Unit tested
release_notes.md updated

codecov-commenter · 2020-05-29T15:37:34Z

Codecov Report

Merging #826 into master will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #826      +/-   ##
============================================
- Coverage     81.27%   81.23%   -0.05%     
+ Complexity      641      637       -4     
============================================
  Files           166      167       +1     
  Lines          3237     3240       +3     
  Branches        463      463              
============================================
+ Hits           2631     2632       +1     
- Misses          342      343       +1     
- Partials        264      265       +1

adamfilipow92 · 2020-05-29T15:44:42Z

Now i have added fast fail for environment-variables. In testlab docs I cannot find any information about unsupported params in robo test. Maybe anyone have more informations?
@pawelpasterz @jan-gogo @bootstraponline @piotradamczyk5

bootstraponline · 2020-05-29T16:40:37Z

Now i have added fast fail for environment-variables. In testlab docs I cannot find any information about unsupported params in robo test. Maybe anyone have more informations?

That's a great question. I think it's mostly trial and error at this point. We could also ask on #test-lab in the https://firebase.community/ Slack.

bootstraponline · 2020-05-30T06:06:49Z

test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt

@@ -1362,6 +1362,24 @@ AndroidArgs
            )
        }
    }
+
+    @Test(expected = FlankFatalError::class)
+    fun `should fast fail wen robo test have env variables set`() {


Suggested change

fun `should fast fail wen robo test have env variables set`() {

fun `should fast fail when the robo test has env variables set`() {

bootstraponline · 2020-05-30T06:07:26Z

test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt

@@ -131,7 +131,9 @@ class AndroidArgs(
        if (roboDirectives.isNotEmpty() && roboScript != null) throw FlankFatalError(
            "Options robo-directives and robo-script are mutually exclusive, use only one of them."
        )
-
+        if (isRoboTest && environmentVariables.isNotEmpty()) throw FlankFatalError(
+            "Environment Variables not supported on Android Robo tests"


Suggested change

"Environment Variables not supported on Android Robo tests"

"Environment variables are not currently supported on Android Robo tests"

bootstraponline · 2020-05-30T06:08:47Z

release_notes.md

@@ -10,7 +10,7 @@
 - [#812](https://github.com/Flank/flank/issues/812) Convert bitrise macOS workflow to github action. ([piotradamczyk5](https://github.com/piotradamczyk5))
 - [#799](https://github.com/Flank/flank/pull/799) Refactor Shared object by splitting it into smaller functions. ([piotradamczyk5](https://github.com/piotradamczyk5))
 - [#798](https://github.com/Flank/flank/pull/798) Remove failure nodes from tests that passed on retry so that Jenkins JUnit plugin marks them as successful. ([adamfilipow92](https://github.com/adamfilipow92))
- 
+- [#826](https://github.com/Flank/flank/pull/826) Fast fail for unsupported params in robo test. ([adamfilipow92](https://github.com/adamfilipow92))


Suggested change

- [#826](https://github.com/Flank/flank/pull/826) Fast fail for unsupported params in robo test. ([adamfilipow92](https://github.com/adamfilipow92))

- [#826](https://github.com/Flank/flank/pull/826) Fast fail when using `environment-variables` in a robo test. ([adamfilipow92](https://github.com/adamfilipow92))

adamfilipow92 · 2020-06-01T12:37:31Z

Now i have added fast fail for environment-variables. In testlab docs I cannot find any information about unsupported params in robo test. Maybe anyone have more informations?

That's a great question. I think it's mostly trial and error at this point. We could also ask on #test-lab in the https://firebase.community/ Slack.

Today, I checked #test-lab and I seen your thread. We got information about rest of TestSetup variables. "All of the other options in TestSetup etc should work for any test type." So maybe we should dedicate this pull request for environment-variables and if in thread we get more informations we make new issue. Additionally I checked this docs: https://firebase.google.com/docs/test-lab/reference/testing/rest/v1/projects.testMatrices#environmentvariable
I can't found more informations about unsupported variables in any test type.

Let me know what do you think.
@bootstraponline

pawelpasterz · 2020-06-01T16:13:19Z

@bootstraponline
With #817 flank gained ability to run both robo and instrumentation tests within one run. With this fail fast approach this feature will impossible to use if one wants to have env variables. Is it something we agree on? (I think it is more like design decision)

jan-goral · 2020-06-01T16:25:16Z

@bootstraponline
With #817 flank gained ability to run both robo and instrumentation tests within one run. With this fail fast approach this feature will impossible to use if one wants to have env variables. Is it something we agree on? (I think it is more like design decision)

Maybe it's possible to just omit env variables for robo test executions instead of throwing exception? Exceptions are good when there are missing data but here we have additional data that is probably not needed for robo tests.

pawelpasterz · 2020-06-01T16:41:37Z

Maybe it's possible to just omit env variables for robo test executions instead of throwing exception? Exceptions are good when there are missing data but here we have additional data that is probably not needed for robo tests.

That was my idea as well, +1

bootstraponline · 2020-06-02T02:46:29Z

Maybe it's possible to just omit env variables for robo test executions instead of throwing exception? Exceptions are good when there are missing data but here we have additional data that is probably not needed for robo tests.

That makes sense to me. In the future, FTL may add env variable support for Robo tests. At that time, we'll want to remove this.

jan-goral · 2020-06-02T20:08:33Z

test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt

@@ -131,7 +131,9 @@ class AndroidArgs(
        if (roboDirectives.isNotEmpty() && roboScript != null) throw FlankFatalError(
            "Options robo-directives and robo-script are mutually exclusive, use only one of them."
        )
-
+        if (isRoboTest && environmentVariables.isNotEmpty()) throw FlankFatalError(


omit env variables for robo test executions instead of throwing exception

jan-goral · 2020-06-03T16:31:49Z

test_runner/src/main/kotlin/ftl/gc/GcAndroidTestMatrix.kt

+    internal fun TestSetup.setEnvironmentVariables(args: AndroidArgs, testConfig: AndroidTestConfig) = this.apply {
+        environmentVariables = if (args.environmentVariables.isNotEmpty() && testConfig is AndroidTestConfig.Instrumentation) {
+            args.environmentVariables.map { it.toEnvironmentVariable() }
+        } else {


braces are redundant for this if-else expression

jan-goral · 2020-06-03T16:34:02Z

test_runner/src/main/kotlin/ftl/gc/GcAndroidTestMatrix.kt

@@ -28,6 +29,15 @@ object GcAndroidTestMatrix {
        value = [email protected]
    }

+    @VisibleForTesting
+    internal fun TestSetup.setEnvironmentVariables(args: AndroidArgs, testConfig: AndroidTestConfig) = this.apply {
+        environmentVariables = if (args.environmentVariables.isNotEmpty() && testConfig is AndroidTestConfig.Instrumentation) {


args.environmentVariables.isNotEmpty() is redundant check, because is nothing wrong with running args.environmentVariables.map { it.toEnvironmentVariable() } on empty args

jan-goral · 2020-06-03T16:37:09Z

test_runner/src/main/kotlin/ftl/gc/GcAndroidTestMatrix.kt

@@ -28,6 +29,15 @@ object GcAndroidTestMatrix {
        value = [email protected]
    }

+    @VisibleForTesting
+    internal fun TestSetup.setEnvironmentVariables(args: AndroidArgs, testConfig: AndroidTestConfig) = this.apply {


This extension deserves for dedicated file. Check out setupAndroidTest for example.

piotradamczyk5 · 2020-06-04T04:28:16Z

test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt

        flank:
-          max-test-shards: ${AVAILABLE_SHARD_COUNT_RANGE.last}
+          max-test-shards: 50


it should be merged with latest master

adamfilipow92 changed the title ~~Add test case and fast fail on env filled on robo test~~ Fast fail on env filled on robo test May 29, 2020

adamfilipow92 marked this pull request as draft May 29, 2020 15:37

adamfilipow92 changed the title ~~Fast fail on env filled on robo test~~ Better validation for unsupported params in robo test May 29, 2020

adamfilipow92 changed the title ~~Better validation for unsupported params in robo test~~ Fast fail for unsupported params in robo test May 29, 2020

bootstraponline reviewed May 30, 2020

View reviewed changes

adamfilipow92 force-pushed the #816-fast-fail-on-unsupported-robo-params branch from 04fd071 to ba0492d Compare June 1, 2020 12:05

adamfilipow92 changed the title ~~Fast fail for unsupported params in robo test~~ Fast fail when using environment-variables in a robo test. Jun 1, 2020

adamfilipow92 marked this pull request as ready for review June 1, 2020 12:41

adamfilipow92 changed the title ~~Fast fail when using environment-variables in a robo test.~~ #816 Fast fail when using environment-variables in a robo test. Jun 1, 2020

adamfilipow92 requested review from jan-goral, pawelpasterz and piotradamczyk5 June 1, 2020 14:13

jan-goral assigned adamfilipow92 Jun 1, 2020

jan-goral suggested changes Jun 2, 2020

View reviewed changes

adamfilipow92 force-pushed the #816-fast-fail-on-unsupported-robo-params branch from 48fdf07 to a390983 Compare June 3, 2020 13:43

adamfilipow92 changed the title ~~#816 Fast fail when using environment-variables in a robo test.~~ #816 Omit environment-variables for robo test executions instead of throwing exception. Jun 3, 2020

adamfilipow92 requested review from bootstraponline and jan-goral June 3, 2020 14:51

jan-goral reviewed Jun 3, 2020

View reviewed changes

piotradamczyk5 reviewed Jun 4, 2020

View reviewed changes

adamfilipow92 added 6 commits June 4, 2020 13:34

Add test case and fast fail on env filled on robo test

85b4109

If robo test not set env variables.

43ed978

Tests, set env variables only on Instrumental tests

d5f7109

Update release_notes.md

8dda809

Cr changes

a75e31f

Update GcAndroidTestMatrixTest.kt

6dfc957

adamfilipow92 force-pushed the #816-fast-fail-on-unsupported-robo-params branch from 9c1c8c7 to 6dfc957 Compare June 4, 2020 11:36

adamfilipow92 added 3 commits June 4, 2020 13:36

Update AndroidArgsTest.kt

6ea45de

Check fails changes

70c81a7

Update release_notes.md

0932f2b

adamfilipow92 requested review from jan-goral and piotradamczyk5 June 4, 2020 11:51

jan-goral approved these changes Jun 4, 2020

View reviewed changes

adamfilipow92 merged commit 81303e5 into master Jun 4, 2020

adamfilipow92 deleted the #816-fast-fail-on-unsupported-robo-params branch June 4, 2020 12:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#816 Omit `environment-variables` for robo test executions instead of throwing exception. #826

#816 Omit `environment-variables` for robo test executions instead of throwing exception. #826

adamfilipow92 commented May 29, 2020 •

edited

Loading

codecov-commenter commented May 29, 2020 •

edited

Loading

adamfilipow92 commented May 29, 2020

bootstraponline commented May 29, 2020

bootstraponline May 30, 2020

bootstraponline May 30, 2020

bootstraponline May 30, 2020

adamfilipow92 commented Jun 1, 2020 •

edited

Loading

pawelpasterz commented Jun 1, 2020 •

edited

Loading

jan-goral commented Jun 1, 2020 •

edited

Loading

pawelpasterz commented Jun 1, 2020 •

edited

Loading

bootstraponline commented Jun 2, 2020

jan-goral Jun 2, 2020

jan-goral Jun 3, 2020

jan-goral Jun 3, 2020

jan-goral Jun 3, 2020

piotradamczyk5 Jun 4, 2020

	fun `should fast fail wen robo test have env variables set`() {
	fun `should fast fail when the robo test has env variables set`() {

	"Environment Variables not supported on Android Robo tests"
	"Environment variables are not currently supported on Android Robo tests"

	- [#826](https://github.com/Flank/flank/pull/826) Fast fail for unsupported params in robo test. ([adamfilipow92](https://github.com/adamfilipow92))
	- [#826](https://github.com/Flank/flank/pull/826) Fast fail when using `environment-variables` in a robo test. ([adamfilipow92](https://github.com/adamfilipow92))

#816 Omit environment-variables for robo test executions instead of throwing exception. #826

#816 Omit environment-variables for robo test executions instead of throwing exception. #826

Conversation

adamfilipow92 commented May 29, 2020 • edited Loading

Checklist

codecov-commenter commented May 29, 2020 • edited Loading

Codecov Report

adamfilipow92 commented May 29, 2020

bootstraponline commented May 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamfilipow92 commented Jun 1, 2020 • edited Loading

pawelpasterz commented Jun 1, 2020 • edited Loading

jan-goral commented Jun 1, 2020 • edited Loading

pawelpasterz commented Jun 1, 2020 • edited Loading

bootstraponline commented Jun 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

#816 Omit `environment-variables` for robo test executions instead of throwing exception. #826

#816 Omit `environment-variables` for robo test executions instead of throwing exception. #826

adamfilipow92 commented May 29, 2020 •

edited

Loading

codecov-commenter commented May 29, 2020 •

edited

Loading

adamfilipow92 commented Jun 1, 2020 •

edited

Loading

pawelpasterz commented Jun 1, 2020 •

edited

Loading

jan-goral commented Jun 1, 2020 •

edited

Loading

pawelpasterz commented Jun 1, 2020 •

edited

Loading