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: Downloading performance Metrics #1326

Merged
merged 8 commits into from
Nov 18, 2020

Conversation

piotradamczyk5
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 commented Nov 17, 2020

Fixes #1324

Test Plan

How do we know the code works?

Scenario A:

  1. Run flank on Android physical device. For example with configuration entry:
gcloud:
...
  device:
    - model: OnePlus6T
      version: 28
...
  1. Check that performanceMetrics.json is stored on the local results device

Scenario B:

  1. Run flank on Android physical device. For example with configuration entry:
gcloud:
...
  device:
    - model: OnePlus6T
      version: 28
...
  1. Specify also file to download from GCloud bucket. For example with configuration entry:
flank:
...
  files-to-download:
    - .*\.json$
...
  1. Check that performanceMetrics.json is downloaded on the local results device

Checklist

  • Save performanceMetrics.json by default to local results directory
  • Fix downloading after tests run
  • Fix local storage directory structure
  • Unit tested

@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2020

Timestamp: 2020-11-18 15:51:24
Buildscan url for ubuntu-workflow run 370523557
https://gradle.com/s/smdplwf7y7dfq

@bootstraponline bootstraponline force-pushed the #1324_fix_downloading_results_for_performanceMetrics branch from 272be86 to f31e885 Compare November 17, 2020 18:03
@piotradamczyk5
Copy link
Contributor Author

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Nov 17, 2020

Command rebase: success

Branch already up to date

}
}
.awaitAll()
}

private fun PerfMetricsSummary.save(resultsDir: String, args: IArgs) {
val configFilePath = if (args.useLocalResultDir())
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only cosmetic but i think smth like

    val configFilePath =
        if (args.useLocalResultDir()) Paths.get(args.localResultDir, "performanceMetrics.json")
        else Paths.get(args.localResultDir, resultsDir, "performanceMetrics.json")

is little more readable. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

@adamfilipow92
Copy link
Contributor

So in both scenarios performanceMetrics.json should be downloaded to local storage, right?

…r_performanceMetrics' into #1324_fix_downloading_results_for_performanceMetrics
@piotradamczyk5
Copy link
Contributor Author

So in both scenarios performanceMetrics.json should be downloaded to local storage, right?

yes

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.

  • Both scenarios tested
    Great work! 👍

@codecov-io
Copy link

codecov-io commented Nov 18, 2020

Codecov Report

Merging #1326 (fb9b70a) into master (733e444) will decrease coverage by 0.03%.
The diff coverage is 71.42%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1326      +/-   ##
============================================
- Coverage     79.61%   79.58%   -0.04%     
- Complexity      735      736       +1     
============================================
  Files           237      237              
  Lines          4573     4579       +6     
  Branches        792      796       +4     
============================================
+ Hits           3641     3644       +3     
+ Misses          526      525       -1     
- Partials        406      410       +4     

@mergify mergify bot merged commit 415330a into master Nov 18, 2020
@mergify mergify bot deleted the #1324_fix_downloading_results_for_performanceMetrics branch November 18, 2020 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot download performanceMetrics.json using "files-to-download" option
5 participants