refactor: Prepare flank codebase for supporting iOS testplans #1281

jan-goral · 2020-10-28T11:57:30Z

This PR brings the partial implementation of #685.

As feature #685 will bring a lot of changes in codebase it will be delivered in separate pull requests for reducing code review complexity.

The main goals of this PR are:

Refactor of existing code related to xctestrun v1, so the code can be reused for v2.
Raw Implementation for xctest v2 (without integration with rest of code)
Preparing the codebase for supporting xctest v2

This pull request SHOULD NOT change the current behavior of the flank.
This pull request IS NOT exposing a new feature.
The full integration of flank with xctestrun v2 will be added in the next pull request(s)

Test Plan

How do we know the code works?

Everything is working like before
The unit tests pass

Checklist

Documented
Refactor of ios Parse.kt file & related unit tests
Refactor of ios Xctestrun.kt file & related unit tests
Test plan (xctestrun v2) parser implementation
Upload test artifacts for #685-ios-support-for-testplans
Unit tested

docs/feature/ios_test_plans.md

test_runner/src/main/kotlin/ftl/ios/xctest/FindTestNames.kt

test_runner/src/main/kotlin/ftl/ios/xctest/RewriteXcTestRun.kt

github-actions · 2020-11-04T13:34:34Z

Timestamp: 2020-11-19 11:40:11
Buildscan url for ubuntu-workflow run 372174738
https://gradle.com/s/fzh35qsqjp5qs

codecov-io · 2020-11-04T13:36:20Z

Codecov Report

Merging #1281 (17be78e) into master (733e444) will decrease coverage by 0.14%.
The diff coverage is 76.71%.

@@             Coverage Diff              @@
##             master    #1281      +/-   ##
============================================
- Coverage     79.61%   79.47%   -0.15%     
+ Complexity      735      700      -35     
============================================
  Files           237      244       +7     
  Lines          4573     4618      +45     
  Branches        792      809      +17     
============================================
+ Hits           3641     3670      +29     
- Misses          526      531       +5     
- Partials        406      417      +11

jan-goral · 2020-11-05T11:35:00Z

After merging #1300 this PR should be easier to review.

Sloox · 2020-11-10T11:58:22Z

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

@@ -79,7 +79,7 @@ IosArgs
 private fun IosArgs.calculateShardChunks() = if (disableSharding)
    emptyList() else
    ArgsHelper.calculateShards(
-        filteredTests = filterTests(findTestNames(xctestrunFile), testTargets)
+        filteredTests = filterTests(findXcTestNamesV1(xctestrunFile), testTargets)


will there be a v2?

Probably but in the next PR (I am working on it). It's in the description "The full integration of flank with xctestrun v2 will be added in the next pull request(s)".

Sloox · 2020-11-10T11:58:58Z

test_runner/src/main/kotlin/ftl/ios/xctest/FindXcTestNamesV2.kt

+import ftl.ios.xctest.common.parseToNSDictionary
+import java.io.File
+
+internal fun findXcTestNamesV2(xctestrun: String): Map<String, XctestrunMethods> =


ahh i see it now. Why the v1 vs v2?

The xctestrun file can come (currently) in two versions. Those files can have a slightly different structure and require different parsers. In the documentation you can find differences between them https://github.com/Flank/flank/blob/4c3b55d30723aefc9ca65b1bec4415a095217531/docs/feature/ios_test_plans.md

Sloox · 2020-11-10T12:00:04Z

test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseObjTests.kt

+    // https://github.com/linkedin/bluepill/blob/37e7efa42472222b81adaa0e88f2bd82aa289b44/Source/Shared/BPXCTestFile.m#L18
+    // must quote binary path in case there are spaces
+    var cmd = "nm -U ${binary.quote()}"
+    if (!FtlConstants.isMacOS) cmd = "PATH=~/.flank $cmd"


has this been tested on windows?

its gonna break it i see it 🗡️

parseObjcTests is used only on ios tests. Flank not supporting ios tests on windows so I think it's fine.

Sloox · 2020-11-10T12:00:50Z

test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseObjTests.kt

+    val results = mutableListOf<String>()
+    // https://github.com/linkedin/bluepill/blob/37e7efa42472222b81adaa0e88f2bd82aa289b44/Source/Shared/BPXCTestFile.m#L18
+    // must quote binary path in case there are spaces
+    var cmd = "nm -U ${binary.quote()}"


i am not sure windows is supported?

Me too, this code is untouched, only moved from:
https://github.com/Flank/flank/blob/ab55fbcb02f6cfe99b3b502db6bf52f661cd34ca/test_runner/src/main/kotlin/ftl/ios/Parse.kt

Everything should work like before. It's in the description: "This pull request SHOULD NOT change the current behavior of the flank."

hmmm alright. We may need to test it

Sloox · 2020-11-10T12:02:25Z

test_runner/src/test/kotlin/ftl/ios/xctest/common/FindTestsForTargetKtTest.kt

+
+    @Test
+    fun `findTestNames respects skip`() {
+        assumeFalse(FtlConstants.isWindows)


we have this here, but what about the other tests?

FYI, most of the tests in this PR are moved from

https://github.com/Flank/flank/blob/ab55fbcb02f6cfe99b3b502db6bf52f661cd34ca/test_runner/src/test/kotlin/ftl/ios/XctestrunTest.kt and

https://github.com/Flank/flank/blob/ab55fbcb02f6cfe99b3b502db6bf52f661cd34ca/test_runner/src/test/kotlin/ftl/ios/ParseTest.kt

to other files to match the refactor of associated functions.

The new tests which I have added are

https://github.com/Flank/flank/blob/4c3b55d30723aefc9ca65b1bec4415a095217531/test_runner/src/test/kotlin/ftl/ios/xctest/FindXcTestNamesV2KtTest.kt

https://github.com/Flank/flank/blob/4c3b55d30723aefc9ca65b1bec4415a095217531/test_runner/src/test/kotlin/ftl/ios/xctest/RewriteXcTestRunV2KtTest.kt

I have no windows to test it so I have followed convention with assumeFalse(FtlConstants.isWindows)

makes sense. We will need a fixup task for windows again.

We will need a fixup task for windows again.

"Again" in this context sounds like that was working previously, then was broken, fixed and now is broken again. But It's not true for this test.

As I said, nothing changes here except file name.

zuziaka · 2020-11-12T09:39:40Z

test_runner/src/test/kotlin/ftl/ios/xctest/common/UtilKtTest.kt

+        val result = parseToNSDictionary(swiftXcTestRunV1)
+        assertThat(arrayOf("EarlGreyExampleSwiftTests", "__xctestrun_metadata__")).isEqualTo(result.allKeys())
+        val dict = result["EarlGreyExampleSwiftTests"] as NSDictionary
+        assertThat(dict.count()).isEqualTo(20)


I know that this test isn't new, but I don't think that asserting number of a key:value pairs here is a good idea.
It may change with new version of Xcode.

test_runner/src/test/kotlin/ftl/ios/xctest/common/UtilKtTest.kt

piotradamczyk5 · 2020-11-16T16:08:07Z

test_runner/src/main/kotlin/ftl/ios/xctest/common/InstallParseBinaries.kt

+import ftl.util.copyBinaryResource
+
+internal val installBinaries by lazy {
+    if (!FtlConstants.isMacOS) {


Will it work properly on Windows 🤔 ?

Not sure. It's refactoring ;P. I haven't got windows OS.

probably it should be
if (!FtlConstants.isMacOS && !isWindows)
but if this just moving to new file, you could leave it

piotradamczyk5 · 2020-11-16T16:09:37Z

I think that this more
refactor:
instead of
chore:

adamfilipow92

Tested
Outputs of master and this branch compared and is the same
👍

* added feature doc template * refactored Xctestru * added ios_test_plan.yml

- updating Xctestrun and FindTestNames methods - wip

Co-authored-by: Jan Góral <[email protected]>

jan-goral assigned zuziaka and jan-goral Oct 28, 2020

zuziaka force-pushed the #685-ios-support-for-testplans branch from e587a71 to 38ae7d2 Compare November 2, 2020 10:22

jan-goral commented Nov 2, 2020

View reviewed changes

docs/feature/ios_test_plans.md Outdated Show resolved Hide resolved

docs/feature/ios_test_plans.md Outdated Show resolved Hide resolved

docs/feature/ios_test_plans.md Outdated Show resolved Hide resolved

jan-goral commented Nov 2, 2020

View reviewed changes

test_runner/src/main/kotlin/ftl/ios/xctest/FindTestNames.kt Outdated Show resolved Hide resolved

test_runner/src/main/kotlin/ftl/ios/xctest/RewriteXcTestRun.kt Outdated Show resolved Hide resolved

jan-goral mentioned this pull request Nov 5, 2020

test: Add flank multi test targets example #1300

Merged

jan-goral changed the title ~~#685 ios support for testplans~~ feat: iOS support for testplans Nov 5, 2020

jan-goral added Feature New Option Used to track PR with new configuration option (useful when updating fladle) Tiger 🐯 labels Nov 5, 2020

jan-goral force-pushed the #685-ios-support-for-testplans branch from 390f9c7 to fb7ba02 Compare November 6, 2020 10:34

jan-goral added Documentation Pull request which touches documentation iOS and removed Documentation Pull request which touches documentation labels Nov 6, 2020

jan-goral changed the title ~~feat: iOS support for testplans~~ chore: Prepare flank codebase for supporting iOS testplans Nov 9, 2020

jan-goral marked this pull request as ready for review November 9, 2020 14:56

jan-goral requested review from pawelpasterz, zuziaka, adamfilipow92, piotradamczyk5 and Sloox November 10, 2020 10:34

Sloox suggested changes Nov 10, 2020

View reviewed changes

jan-goral requested a review from Sloox November 10, 2020 13:38

bootstraponline force-pushed the #685-ios-support-for-testplans branch 2 times, most recently from 29bb33f to a97c1aa Compare November 12, 2020 07:00

zuziaka reviewed Nov 12, 2020

View reviewed changes

jan-goral commented Nov 12, 2020

View reviewed changes

test_runner/src/test/kotlin/ftl/ios/xctest/common/UtilKtTest.kt Outdated Show resolved Hide resolved

bootstraponline force-pushed the #685-ios-support-for-testplans branch from 0007791 to 858801f Compare November 16, 2020 12:19

piotradamczyk5 reviewed Nov 16, 2020

View reviewed changes

jan-goral changed the title ~~chore: Prepare flank codebase for supporting iOS testplans~~ refactor: Prepare flank codebase for supporting iOS testplans Nov 17, 2020

bootstraponline force-pushed the #685-ios-support-for-testplans branch from 858801f to d81c933 Compare November 17, 2020 13:02

pawelpasterz requested a review from piotradamczyk5 November 17, 2020 15:42

piotradamczyk5 approved these changes Nov 17, 2020

View reviewed changes

jan-goral force-pushed the #685-ios-support-for-testplans branch from d81c933 to 17be78e Compare November 18, 2020 12:05

adamfilipow92 approved these changes Nov 18, 2020

View reviewed changes

bootstraponline force-pushed the #685-ios-support-for-testplans branch from 17be78e to ee6a99a Compare November 18, 2020 16:04

jan-goral force-pushed the #685-ios-support-for-testplans branch from ee6a99a to cd39b11 Compare November 19, 2020 11:30

zuziaka and others added 15 commits November 19, 2020 11:34

Added sample iOS Project with test plans

075e4c2

* added feature doc template * refactored Xctestru * added ios_test_plan.yml

Rebased from master

0cad40e

- updating Xctestrun and FindTestNames methods - wip

Fixed tests and removed comments

6fe16c0

Update docs/feature/ios_test_plans.md

fc490b8

Co-authored-by: Jan Góral <[email protected]>

Update docs/feature/ios_test_plans.md

d658ddd

Co-authored-by: Jan Góral <[email protected]>

Update ios_test_plans.md

c56092d

Fixed code formatting

d9c2925

Update test_runner/src/main/kotlin/ftl/ios/xctest/FindTestNames.kt

49e650d

Update docs/feature/ios_test_plans.md

0b2283d

Update test_runner/src/main/kotlin/ftl/ios/xctest/RewriteXcTestRun.kt

2fde14e

Add implementation for rewriting xctestrun v2

856ddb8

Revert incorrect pars test value

8362e51

Upgrade xctest package

c840727

Upgrade unit tests & fix rewriteXcTestRunV2 implementation

20d805c

Update test_runner/src/test/kotlin/ftl/ios/xctest/common/UtilKtTest.kt

fa5ff24

bootstraponline force-pushed the #685-ios-support-for-testplans branch from cd39b11 to fa5ff24 Compare November 19, 2020 11:34

Sloox approved these changes Nov 19, 2020

View reviewed changes

jan-goral merged commit 501ea71 into master Nov 19, 2020

jan-goral deleted the #685-ios-support-for-testplans branch November 19, 2020 13:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Prepare flank codebase for supporting iOS testplans #1281

refactor: Prepare flank codebase for supporting iOS testplans #1281

jan-goral commented Oct 28, 2020 •

edited

Loading

github-actions bot commented Nov 4, 2020 •

edited

Loading

codecov-io commented Nov 4, 2020 •

edited

Loading

jan-goral commented Nov 5, 2020

Sloox Nov 10, 2020

jan-goral Nov 10, 2020 •

edited

Loading

Sloox Nov 10, 2020

jan-goral Nov 10, 2020

Sloox Nov 10, 2020

jan-goral Nov 10, 2020

Sloox Nov 11, 2020

adamfilipow92 Nov 17, 2020

Sloox Nov 10, 2020

jan-goral Nov 10, 2020

Sloox Nov 11, 2020

Sloox Nov 10, 2020

jan-goral Nov 10, 2020

Sloox Nov 11, 2020

jan-goral Nov 12, 2020

zuziaka Nov 12, 2020

piotradamczyk5 Nov 16, 2020

jan-goral Nov 17, 2020 •

edited

Loading

piotradamczyk5 Nov 17, 2020

piotradamczyk5 commented Nov 16, 2020 •

edited

Loading

adamfilipow92 left a comment

refactor: Prepare flank codebase for supporting iOS testplans #1281

refactor: Prepare flank codebase for supporting iOS testplans #1281

Conversation

jan-goral commented Oct 28, 2020 • edited Loading

Test Plan

Checklist

github-actions bot commented Nov 4, 2020 • edited Loading

codecov-io commented Nov 4, 2020 • edited Loading

Codecov Report

jan-goral commented Nov 5, 2020

Choose a reason for hiding this comment

jan-goral Nov 10, 2020 • edited Loading

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

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

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

Choose a reason for hiding this comment

jan-goral Nov 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piotradamczyk5 commented Nov 16, 2020 • edited Loading

adamfilipow92 left a comment

Choose a reason for hiding this comment

jan-goral commented Oct 28, 2020 •

edited

Loading

github-actions bot commented Nov 4, 2020 •

edited

Loading

codecov-io commented Nov 4, 2020 •

edited

Loading

jan-goral Nov 10, 2020 •

edited

Loading

jan-goral Nov 17, 2020 •

edited

Loading

piotradamczyk5 commented Nov 16, 2020 •

edited

Loading