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

feat: Merge test method duration for parameterized classes #2062

Merged
merged 7 commits into from
Jul 9, 2021

Conversation

jan-goral
Copy link
Contributor

@jan-goral jan-goral commented Jul 1, 2021

Fixes #2052

Changes

  • Adds to module :tool:junit function for merging test methods into test-class with accumulated duration.
  • Merges test methods for given class names while loading previous durations.

Test Plan

How do we know the code works?

Build flank:

. .env
flankScripts assemble flank -d

Run the following command twice:

flank corellium test android run -c="./test_configs/flank-corellium.yml"

The second run should calculate durations for parameterized classes.
Check generated android-shards.json each parameterized class should have a duration different than the default (120).

Checklist

  • Documented
  • Unit tested

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2021

Timestamp: 2021-07-09 11:26:22
Buildscan url for ubuntu-workflow run 1014981909
https://gradle.com/s/scy6ucprogllm

Sloox
Sloox previously requested changes Jul 2, 2021
Copy link
Contributor

@Sloox Sloox left a comment

Choose a reason for hiding this comment

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

@jan-gogo ignore that. This is for corellium

@Sloox Sloox self-requested a review July 2, 2021 10:14
@jan-goral jan-goral force-pushed the 2052_Parameterized_tests_durations_support branch from 8065ea0 to 1ebb01b Compare July 2, 2021 15:44
@jan-goral jan-goral marked this pull request as ready for review July 6, 2021 11:03
@jan-goral jan-goral dismissed Sloox’s stale review July 6, 2021 11:05

Because of comment "@jan-gogo ignore that. This is for corellium"

@bootstraponline bootstraponline force-pushed the 2052_Parameterized_tests_durations_support branch from 916937d to 1c0863f Compare July 6, 2021 13:01
mergify bot pushed a commit that referenced this pull request Jul 6, 2021
Fixes #2051

Requires #2055

Enchanted by #2062

## Changes

* Add test targets argument for filtering test cases.
* Add filtering option to Apk.ParseTestCases.
* Log AndroidTestPlan.Config on ExecuteTests start.
* Refactor `:tool:apk` module

## Test Plan
> How do we know the code works?

build flank
```shell
. .env
flankScripts assemble flank -d
```
and run 
```shell
flank corellium test android run -c="./test_configs/flank-corellium.yml"
```
using configuration
```yml
auth: "test_configs/corellium_auth.yml"
apks:
  - path: "test_artifacts/master/apk/app-debug.apk"
    tests:
      - path: "test_artifacts/master/apk/app-multiple-flaky-debug-androidTest.apk"
test-targets:
  - "package com.example.test_app.parametrized"
  - "notClass com.example.test_app.parametrized.EspressoParametrizedClassTestParameterized"
  - "package com.example.test_app.foo"
  - "class com.example.test_app.InstrumentedTest#test0"
max-test-shards: 3
```
As a results Flank should execute only test cases restricted by test targets.

## Checklist

- [x] Documented
- [x] Unit tested
- [x] Integrated with a filtering tool
@bootstraponline bootstraponline force-pushed the 2052_Parameterized_tests_durations_support branch 2 times, most recently from f6d8fd2 to b8e186e Compare July 6, 2021 14:44
@jan-goral jan-goral added the P0 label Jul 7, 2021
Copy link
Contributor

@Sloox Sloox left a comment

Choose a reason for hiding this comment

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

Seems alright. Minor comment :)

JUnit.TestResult(
suiteName = suite.name,
testName = case.name,
className = case.classname,
startAt = 0,
endsAt = (case.time * 1000).toLong(),
startAt = startAt.also { startAt = endAt },
Copy link
Contributor

Choose a reason for hiding this comment

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

wont this simply result in
endAt = endAt
&
startAt = endAt?

Copy link
Contributor Author

@jan-goral jan-goral Jul 9, 2021

Choose a reason for hiding this comment

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

I don't understand your suggestion, but it's reminded me that I had forgotten to add a proper test (Thx for that 👍). So, I added a small update. If you see a shorter solution, feel free to check out the branch and test it using StructuralKtTest.mapToTestResultsTest

Copy link
Contributor Author

@jan-goral jan-goral Jul 9, 2021

Choose a reason for hiding this comment

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

Right, this part .also { startAt = endAt } was redundant.

@bootstraponline bootstraponline force-pushed the 2052_Parameterized_tests_durations_support branch 2 times, most recently from bcfc0b9 to 4d9b334 Compare July 9, 2021 07:30
@bootstraponline bootstraponline force-pushed the 2052_Parameterized_tests_durations_support branch from 5af09f6 to 2c7bf3e Compare July 9, 2021 11:20
@mergify mergify bot merged commit 4ce447a into master Jul 9, 2021
@mergify mergify bot deleted the 2052_Parameterized_tests_durations_support branch July 9, 2021 11:43
@github-actions github-actions bot locked and limited conversation to collaborators Jul 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameterized tests support
4 participants