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

#810 Prints matrix error as table #819

Merged
merged 10 commits into from
Jun 1, 2020

Conversation

piotradamczyk5
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 commented May 27, 2020

Fixes #810

Checklist

  • Unit tested
  • release_notes.md updated

// when
withGlobalExceptionHandling(block)
// then
assertTrue(output.log.contains("Error: Matrix failed: 1"))
assertTrue(output.log.contains("Error: Matrix failed: 2"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test will be updated cause assert methods was not called after System.exit, so I introduced new testing behavior given-will-when 😄 to make them work

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for updating this test 👍

@codecov-commenter
Copy link

codecov-commenter commented May 28, 2020

Codecov Report

Merging #819 into master will increase coverage by 0.37%.
The diff coverage is 94.11%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #819      +/-   ##
============================================
+ Coverage     78.30%   78.67%   +0.37%     
- Complexity      667      675       +8     
============================================
  Files           149      152       +3     
  Lines          3014     3090      +76     
  Branches        439      446       +7     
============================================
+ Hits           2360     2431      +71     
- Misses          374      376       +2     
- Partials        280      283       +3     

@piotradamczyk5 piotradamczyk5 changed the title #810 prints matrix error as table #810 Prints matrix error as table May 28, 2020
@piotradamczyk5 piotradamczyk5 marked this pull request as ready for review May 28, 2020 07:01
@jan-goral
Copy link
Contributor

I wonder if #810 is about displaying only error outputs or maybe also successed. @bootstraponline ?

@bootstraponline
Copy link
Contributor

I wonder if #810 is about displaying only error outputs or maybe also successed. @bootstraponline ?

I think we should display the results (failed, success, etc.) for each matrix. Same as gcloud CLI.

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.

Add support for other status types

@piotradamczyk5
Copy link
Contributor Author

Add support for other status types

I have added printing MatrixResultsReport as table

}
val total = matrices.map.size
// unfinished matrix will not be reported as failed since it's still running
val success = matrices.map.values.count { it.failed().not() }
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@jan-goral
Copy link
Contributor

For Debug.kt

    val quantity = "single"
    val type = "error"

Table is printed twice

AndroidArgs
    gcloud:
      results-bucket: test-lab-v9cn46bb990nx-kz69ymd4nm9aq
      results-dir: null
      record-video: false
      timeout: 15m
      async: false
      client-details: 
      network-profile: null
      results-history-name: null
      # Android gcloud
      app: /Users/janek/projects/flank-project/flank-review/test_runner/src/test/kotlin/ftl/fixtures/tmp/apk/app-debug.apk
      test: /Users/janek/projects/flank-project/flank-review/test_runner/src/test/kotlin/ftl/fixtures/tmp/apk/app-single-error-debug-androidTest.apk
      additional-apks: 
      auto-google-login: false
      use-orchestrator: true
      directories-to-pull:
        - /sdcard/
      other-files:
      performance-metrics: false
      num-uniform-shards: null
      test-runner-class: null
      test-targets:
      robo-directives:
      robo-script: null
      device:
        - model: NexusLowRes
          version: 28
          locale: en
          orientation: portrait
      num-flaky-test-attempts: 0

    flank:
      max-test-shards: 1
      shard-time: -1
      num-test-runs: 1
      smart-flank-gcs-path: 
      smart-flank-disable-upload: false
      files-to-download:
        - .*/sdcard/[^/]+\.ec$
      test-targets-always-run:
      disable-sharding: true
      project: flank-open-source
      local-result-dir: results
      # Android Flank Yml
      keep-file-path: false
      additional-app-test-apks:
      run-timeout: -1
      legacy-junit-result: false
      ignore-failed-tests: false
      output-style: verbose

RunTests
  Uploading app-debug.apk   Uploading app-single-error-debug-androidTest.apk ..

  1 test / 1 shard

  1 matrix ids created in 0m 6s
  https://console.developers.google.com/storage/browser/test-lab-v9cn46bb990nx-kz69ymd4nm9aq/2020-05-29_12-06-17.849000_lOtW

Matrices webLink
  matrix-23ywqaa9yt86m https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/8785051465867885324/executions/bs.f52b9341ebd2e7de

  0m  1s matrix-23ywqaa9yt86m NexusLowRes-28 PENDING
  0m 31s matrix-23ywqaa9yt86m NexusLowRes-28 Starting attempt 1.
  0m 31s matrix-23ywqaa9yt86m NexusLowRes-28 RUNNING
  1m 32s matrix-23ywqaa9yt86m NexusLowRes-28 Started logcat recording.
  1m 32s matrix-23ywqaa9yt86m NexusLowRes-28 Preparing device.
  1m 57s matrix-23ywqaa9yt86m NexusLowRes-28 Installing apps.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Retrieving Pre-Test Package Stats information from the device.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Retrieving Performance Environment information from the device.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Started crash detection.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Started crash monitoring.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Starting instrumentation test.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Completed instrumentation test.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Stopped crash monitoring.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Retrieving Post-test Package Stats information from the device.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Stopped logcat recording.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Done. Test time = 6 (secs)
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Starting results processing. Attempt: 1
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Completed results processing. Time taken = 2 (secs)
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 FINISHED
  2m 33s matrix-23ywqaa9yt86m FINISHED

FetchArtifacts
  .
  Updating matrix file

CostReport
  Virtual devices
    $0.02 for 1m

MatrixResultsReport
  0 / 1 (0.00%)
  1 matrices failed

┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-23ywqaa9yt86m  │                    │
└─────────┴────────────────────────┴────────────────────┘
More details are available at [https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/8785051465867885324/executions/bs.f52b9341ebd2e7de]
┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-23ywqaa9yt86m  │                    │
└─────────┴────────────────────────┴────────────────────┘

Process finished with exit code 1

@piotradamczyk5
Copy link
Contributor Author

For Debug.kt

    val quantity = "single"
    val type = "error"

Table is printed twice

AndroidArgs
    gcloud:
      results-bucket: test-lab-v9cn46bb990nx-kz69ymd4nm9aq
      results-dir: null
      record-video: false
      timeout: 15m
      async: false
      client-details: 
      network-profile: null
      results-history-name: null
      # Android gcloud
      app: /Users/janek/projects/flank-project/flank-review/test_runner/src/test/kotlin/ftl/fixtures/tmp/apk/app-debug.apk
      test: /Users/janek/projects/flank-project/flank-review/test_runner/src/test/kotlin/ftl/fixtures/tmp/apk/app-single-error-debug-androidTest.apk
      additional-apks: 
      auto-google-login: false
      use-orchestrator: true
      directories-to-pull:
        - /sdcard/
      other-files:
      performance-metrics: false
      num-uniform-shards: null
      test-runner-class: null
      test-targets:
      robo-directives:
      robo-script: null
      device:
        - model: NexusLowRes
          version: 28
          locale: en
          orientation: portrait
      num-flaky-test-attempts: 0

    flank:
      max-test-shards: 1
      shard-time: -1
      num-test-runs: 1
      smart-flank-gcs-path: 
      smart-flank-disable-upload: false
      files-to-download:
        - .*/sdcard/[^/]+\.ec$
      test-targets-always-run:
      disable-sharding: true
      project: flank-open-source
      local-result-dir: results
      # Android Flank Yml
      keep-file-path: false
      additional-app-test-apks:
      run-timeout: -1
      legacy-junit-result: false
      ignore-failed-tests: false
      output-style: verbose

RunTests
  Uploading app-debug.apk   Uploading app-single-error-debug-androidTest.apk ..

  1 test / 1 shard

  1 matrix ids created in 0m 6s
  https://console.developers.google.com/storage/browser/test-lab-v9cn46bb990nx-kz69ymd4nm9aq/2020-05-29_12-06-17.849000_lOtW

Matrices webLink
  matrix-23ywqaa9yt86m https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/8785051465867885324/executions/bs.f52b9341ebd2e7de

  0m  1s matrix-23ywqaa9yt86m NexusLowRes-28 PENDING
  0m 31s matrix-23ywqaa9yt86m NexusLowRes-28 Starting attempt 1.
  0m 31s matrix-23ywqaa9yt86m NexusLowRes-28 RUNNING
  1m 32s matrix-23ywqaa9yt86m NexusLowRes-28 Started logcat recording.
  1m 32s matrix-23ywqaa9yt86m NexusLowRes-28 Preparing device.
  1m 57s matrix-23ywqaa9yt86m NexusLowRes-28 Installing apps.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Retrieving Pre-Test Package Stats information from the device.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Retrieving Performance Environment information from the device.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Started crash detection.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Started crash monitoring.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Starting instrumentation test.
  2m  3s matrix-23ywqaa9yt86m NexusLowRes-28 Completed instrumentation test.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Stopped crash monitoring.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Retrieving Post-test Package Stats information from the device.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Stopped logcat recording.
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Done. Test time = 6 (secs)
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Starting results processing. Attempt: 1
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 Completed results processing. Time taken = 2 (secs)
  2m 33s matrix-23ywqaa9yt86m NexusLowRes-28 FINISHED
  2m 33s matrix-23ywqaa9yt86m FINISHED

FetchArtifacts
  .
  Updating matrix file

CostReport
  Virtual devices
    $0.02 for 1m

MatrixResultsReport
  0 / 1 (0.00%)
  1 matrices failed

┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-23ywqaa9yt86m  │                    │
└─────────┴────────────────────────┴────────────────────┘
More details are available at [https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/8785051465867885324/executions/bs.f52b9341ebd2e7de]
┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-23ywqaa9yt86m  │                    │
└─────────┴────────────────────────┴────────────────────┘

Process finished with exit code 1

Basically the first table is a summary report for all matrix (if you have many matrices, it will print them all) and second one log error of each matrix.

To fix this issue I could skip printing table as error log in Utils.kt#withGlobalExceptionHandling this is much decision about how output should look like.

Piotr Adamczyk added 2 commits May 29, 2020 14:24
@bootstraponline
Copy link
Contributor

Basically the first table is a summary report for all matrix (if you have many matrices, it will print them all) and second one log error of each matrix.

To fix this issue I could skip printing table as error log in Utils.kt#withGlobalExceptionHandling this is much decision about how output should look like.

I'd study the gcloud CLI matrix table. We should be able to port that to Kotlin.
https://github.com/Flank/gcloud_cli/search?q=TEST_AXIS_VALUE&unscoped_q=TEST_AXIS_VALUE

The end result should be only one table. Primarily the goal of Flank is to match gcloud, and add a bit of extra features on top. 🙂

@bootstraponline
Copy link
Contributor

Here's one way you can test the PR:

Run flank.yml with Flank.

  • flank android run

Run the same flank.yml with gcloud. The table output should match. 🙂

  • gcloud firebase test android run flank.yml:gcloud

@piotradamczyk5
Copy link
Contributor Author

piotradamczyk5 commented Jun 1, 2020

I made output table same as for GCloud ( link, single table, status color etc)
I used outcomeDetails to print TEST_DETAILS, but it is usually empty. Should I use other data to print test details? Outcome details in my opinion should be good for it

@adamfilipow92
Copy link
Contributor

I will check outputs
Flank table:

┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-d587bp9k4fcda  │                    │
└─────────┴────────────────────────┴────────────────────┘

and Test Lab

┌─────────┬────────────────────────┬─────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │     TEST_DETAILS    │
├─────────┼────────────────────────┼─────────────────────┤
│ Failed  │ walleye-26-en-portrait │ 1 test cases failed │
└─────────┴────────────────────────┴─────────────────────┘

Colors are the same. But in flank we don't have TEST_DETAILS filled. I don't known its ok so please tell what you think. @bootstraponline @pawelpasterz @jan-gogo

@piotradamczyk5
Copy link
Contributor Author

piotradamczyk5 commented Jun 1, 2020

I will check outputs
Flank table:

┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-d587bp9k4fcda  │                    │
└─────────┴────────────────────────┴────────────────────┘

and Test Lab

┌─────────┬────────────────────────┬─────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │     TEST_DETAILS    │
├─────────┼────────────────────────┼─────────────────────┤
│ Failed  │ walleye-26-en-portrait │ 1 test cases failed │
└─────────┴────────────────────────┴─────────────────────┘

Colors are the same. But in flank we don't have TEST_DETAILS filled. I don't known its ok so please tell what you think. @bootstraponline @pawelpasterz @jan-gogo

Yes, that's the question which I asked above about properly data usage

I made output table same as for GCloud ( link, single table, status color etc)
I used outcomeDetails to print TEST_DETAILS, but it is usually empty. Should I use other data to print test details? Outcome details in my opinion should be good for it

pawelpasterz
pawelpasterz previously approved these changes Jun 1, 2020
// when
withGlobalExceptionHandling(block)
// then
assertTrue(output.log.contains("Error: Matrix failed: 1"))
assertTrue(output.log.contains("Error: Matrix failed: 2"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for updating this test 👍

@pawelpasterz
Copy link
Contributor

pawelpasterz commented Jun 1, 2020

I will check outputs
Flank table:

┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-d587bp9k4fcda  │                    │
└─────────┴────────────────────────┴────────────────────┘

and Test Lab

┌─────────┬────────────────────────┬─────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │     TEST_DETAILS    │
├─────────┼────────────────────────┼─────────────────────┤
│ Failed  │ walleye-26-en-portrait │ 1 test cases failed │
└─────────┴────────────────────────┴─────────────────────┘

Colors are the same. But in flank we don't have TEST_DETAILS filled. I don't known its ok so please tell what you think. @bootstraponline @pawelpasterz @jan-gogo

Yes, that's the question which I asked above about properly data usage

I made output table same as for GCloud ( link, single table, status color etc)
I used outcomeDetails to print TEST_DETAILS, but it is usually empty. Should I use other data to print test details? Outcome details in my opinion should be good for it

@adamfilipow92
IMO we should try to fix it but in separate issue/PR. If we want to reflect gcloud results I think there would be some refactor needed (small PR scope principle)
https://github.com/Flank/gcloud_cli/blob/cca7556cc832be02b3523c0fe86fdbdcd33078c4/google-cloud-sdk/lib/googlecloudsdk/api_lib/firebase/test/results_summary.py#L326

@piotradamczyk5
Copy link
Contributor Author

I will check outputs
Flank table:

┌─────────┬────────────────────────┬────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │    TEST_DETAILS    │
├─────────┼────────────────────────┼────────────────────┤
│ failure │  matrix-d587bp9k4fcda  │                    │
└─────────┴────────────────────────┴────────────────────┘

and Test Lab

┌─────────┬────────────────────────┬─────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │     TEST_DETAILS    │
├─────────┼────────────────────────┼─────────────────────┤
│ Failed  │ walleye-26-en-portrait │ 1 test cases failed │
└─────────┴────────────────────────┴─────────────────────┘

Colors are the same. But in flank we don't have TEST_DETAILS filled. I don't known its ok so please tell what you think. @bootstraponline @pawelpasterz @jan-gogo

Yes, that's the question which I asked above about properly data usage

I made output table same as for GCloud ( link, single table, status color etc)
I used outcomeDetails to print TEST_DETAILS, but it is usually empty. Should I use other data to print test details? Outcome details in my opinion should be good for it

@adamfilipow92
IMO we should try to fix it but in separate issue/PR. If we want to reflect gcloud results I think there would be some refactor needed (small PR scope principle)
https://github.com/Flank/gcloud_cli/blob/cca7556cc832be02b3523c0fe86fdbdcd33078c4/google-cloud-sdk/lib/googlecloudsdk/api_lib/firebase/test/results_summary.py#L326

Should I create a task for it?

@jan-goral
Copy link
Contributor

jan-goral commented Jun 1, 2020

@piotradamczyk5

IMO we should try to fix it but in separate issue/PR. If we want to reflect gcloud results I think there would be some refactor needed (small PR scope principle)

I guess It's a good idea If you feel more comfortable with separated issue/PR

@piotradamczyk5 piotradamczyk5 merged commit c88cb27 into master Jun 1, 2020
@piotradamczyk5 piotradamczyk5 deleted the #810-added-display-matrix-error-as-table branch June 1, 2020 15:56
@piotradamczyk5
Copy link
Contributor Author

@piotradamczyk5

IMO we should try to fix it but in separate issue/PR. If we want to reflect gcloud results I think there would be some refactor needed (small PR scope principle)

I guess It's a good idea If you feel more comfortable with separated issue/PR

Created #829

@bootstraponline
Copy link
Contributor

I agree, let's match gcloud output. Having this as a new task makes sense. #829 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display matrix results in a table format
6 participants