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.

Sign up for GitHub

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

Jump to bottom

Handle duplicated apks names #854

Closed
wants to merge 6 commits into from
Closed

Handle duplicated apks names #854

wants to merge 6 commits into from

Conversation

pawelpasterz
Copy link
Contributor

@pawelpasterz pawelpasterz commented Jun 16, 2020 •
edited
Loading

Potential fix for #818

Test Plan

How do we know the code works?

All tests are correctly launched when additional test apks have the same name. Appended numbers to test apks names correspond with matrix_[number].
Robo test matrix directory has new name: matrix_robo_[number].
Instrumentation and robo matrices have separated counters.
Screenshot 2020-06-17 at 10 10 04

flank.yml

gcloud:
  app: ~/duplicated_names/app-debug.apk
  test: ~/duplicated_names/dir1/app-multiple-success-debug-androidTest.apk
  use-orchestrator: false
flank:
  disable-sharding: false
  max-test-shards: 3
  additional-app-test-apks:
    - test: ~/duplicated_names/dir2/app-multiple-success-debug-androidTest.apk
    - test: ~/duplicated_names/dir3/app-multiple-success-debug-androidTest.apk
    - test: ~/duplicated_names/dir4/app-multiple-success-debug-androidTest.apk

++++ Before fix ++++

Bucket (note only one test apk):
gcs bucket
Screenshot 2020-06-17 at 08 55 26

Matrices:
matrix-3czoyl2reb5zb -- SUCCESS
matrix-1syf6utonh6av -- FAILED
matrix-1ih4gd2jimkhi -- FAILED
matrix-mgl0d9m5f0oia -- FAILED
Screenshot 2020-06-17 at 08 38 18
Screenshot 2020-06-17 at 08 38 08

++++ After fix ++++

Bucket (multiple test apk with suffix _[number].apk):
gcs bucket
Screenshot 2020-06-17 at 08 54 03
Matrices:
matrix-67npvezinaqva -- SUCCESS
matrix-20mfp8bhyycmh -- SUCCESS
matrix-3vfuw3av7vxld -- SUCCESS
matrix-11a8n4gbhe2i0 -- SUCCESS
JUnitResults.xml
JUnitReport.zip

Checklist

  • Documented
  • Unit tested
  • release_notes.md updated

@pawelpasterz pawelpasterz force-pushed the handle-duplicated-apks-names branch from 7aa3e78 to 9c226ff Compare June 16, 2020 06:35
@pawelpasterz pawelpasterz force-pushed the handle-duplicated-apks-names branch from 83e8487 to 733b5a0 Compare June 17, 2020 09:05
@pawelpasterz pawelpasterz force-pushed the handle-duplicated-apks-names branch from 7b00daa to 23e059c Compare June 17, 2020 09:22
adamfilipow92
adamfilipow92 previously approved these changes Jun 17, 2020
View reviewed changes
Copy link
Contributor

@adamfilipow92 adamfilipow92 left a comment

Choose a reason for hiding this comment

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

👍

@piotradamczyk5 piotradamczyk5 self-requested a review June 17, 2020 13:02
piotradamczyk5
piotradamczyk5 previously approved these changes Jun 17, 2020
View reviewed changes
jan-goral
jan-goral reviewed Jun 17, 2020
View reviewed changes
Copy link
Contributor

@jan-goral jan-goral left a comment

Choose a reason for hiding this comment

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

The implementation looks great. Now will be nice to have dedicated yml config and bundle with apks (probably in fixtures) to easly reproduce this case. Also we need unit test for file name conversion.

@adamfilipow92 adamfilipow92 dismissed stale reviews from piotradamczyk5 and themself via d5ba692 June 17, 2020 16:44
piotradamczyk5
piotradamczyk5 reviewed Jun 18, 2020
View reviewed changes
test_runner/src/main/kotlin/ftl/gc/GcStorage.kt Outdated
@@ -22,7 +23,7 @@ import java.nio.file.Paths
import java.util.concurrent.ConcurrentHashMap

object GcStorage {

@VisibleForTesting
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this is not true

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I tested something and forgot remove it before commit. Thanks!

piotradamczyk5
piotradamczyk5 reviewed Jun 18, 2020
View reviewed changes
test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt Outdated
@@ -73,3 +77,13 @@ private suspend fun executeAndroidTestMatrix(
}
}
}

private val regex = ".*_(\\d)\\.apk".toRegex()
Copy link
Contributor

Choose a reason for hiding this comment

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

regex -> apkFileRegex

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, done!

@pawelpasterz
Copy link
Contributor Author

Enhanced solution presented in #858

@bootstraponline bootstraponline deleted the handle-duplicated-apks-names branch November 12, 2021 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jan-goral jan-goral jan-goral left review comments

@adamfilipow92 adamfilipow92 adamfilipow92 left review comments

@piotradamczyk5 piotradamczyk5 piotradamczyk5 left review comments

Assignees

@pawelpasterz pawelpasterz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pawelpasterz @jan-goral @adamfilipow92 @piotradamczyk5