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: #842 support for test targets flag in multiple testbundles xctest #1219

Conversation

zuziaka
Copy link
Contributor

@zuziaka zuziaka commented Oct 6, 2020

Fixes #842

Describe the bug

If the original .xctestrun has multiple XCTest entries, Tests included via --test-targets flag will be added to the OnlyTestIdentifiers array on every XCTest entry, not just the one(s) that actually includes that test.
__
Currently Flank rewrites whole .xctestrun file with methods specified --test-targets without checking whether those methods actually exists in XCTest. This solution works fine for a .xctestrun files containing only one XCTest.

Solution

Flank now rewrites .xctestrun with mapping correct methods names with specific XCTests.

Test Plan

How do we know the code works?

  • unit tested
  • new test artifacts with multiple XCTest entries

Checklist

  • Documented
  • Unit tested
  • Test artifacts added to TestProjects

@@ -17,6 +19,8 @@ import java.nio.file.Paths
class XctestrunTest {

private val swiftXctestrun = "$fixturesPath/EarlGreyExampleSwiftTests_iphoneos13.4-arm64e.xctestrun"
private val multipleTargetsSwiftXctestrun = "$fixturesPath/axel/AllTests_iphoneos13.7-arm64e.xctestrun"
Copy link
Contributor Author

@zuziaka zuziaka Oct 6, 2020

Choose a reason for hiding this comment

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

@jan-gogo In order to test new functionality, I have to add some new fixtures, but we should wait for this PR
#1148, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jan-gogo In order to test new functionality, I have to add some new fixtures, but we should wait for this PR
#1148, right?

Right

@zuziaka zuziaka force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch from 83503a6 to 960e9fc Compare October 14, 2020 08:35
@zuziaka zuziaka marked this pull request as ready for review October 15, 2020 11:06
@bootstraponline bootstraponline force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch from d6e5864 to aafdb17 Compare October 15, 2020 11:06
@zuziaka zuziaka requested a review from pawelpasterz October 15, 2020 11:07
@mergify
Copy link

mergify bot commented Oct 15, 2020

Title does not follow the guidelines of Conventional Commits.
Please adjust title before merge and use one of following prefix:

  • build - Changes that affect the build system or external dependencies (dependencies update)
  • ci - Changes to our CI configuration files and scripts (basically directory .github/workflows)
  • docs - Documentation only changes
  • feat - A new feature
  • fix - A bug fix
  • chore - Changes which does not touch the code (ex. manual update of release notes). It will not generate release notes changes
  • refactor - A code change that contains refactor
  • style - Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test - Adding missing tests or correcting existing tests and also changes for our test app
  • perf - A code change that improves performance (I do not think we will use it)

@@ -9,3 +9,4 @@ test_runner/flank/
local.properties
/report.json
results
xcuserdata/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

User generated data for iOS projects (breakpoints, interface state etc.)

@@ -509,9 +509,9 @@
COPY_PHASE_STRIP = NO;
DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym";
DEFINES_MODULE = NO;
DEVELOPMENT_TEAM = AD2V26JBWL;
DEVELOPMENT_TEAM = L2UF9MLSM6;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using GogoApps code signing

@codecov-io
Copy link

codecov-io commented Oct 15, 2020

Codecov Report

Merging #1219 into master will increase coverage by 0.07%.
The diff coverage is 95.12%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1219      +/-   ##
============================================
+ Coverage     79.65%   79.72%   +0.07%     
- Complexity      715      721       +6     
============================================
  Files           233      233              
  Lines          4399     4410      +11     
  Branches        756      757       +1     
============================================
+ Hits           3504     3516      +12     
  Misses          496      496              
+ Partials        399      398       -1     

@zuziaka zuziaka changed the title #842 support for test targets flag in multiple testbundles xctest fix: #842 support for test targets flag in multiple testbundles xctest Oct 15, 2020
@@ -136,7 +136,7 @@ detekt {
input = files("src/main/kotlin", "src/test/kotlin")
config = files("../config/detekt.yml")
parallel = true
failFast = true // fail build on any finding
failFast = false // fail build on any finding
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, changed this for testing - already removed

@bootstraponline bootstraponline force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch 5 times, most recently from 9cf3f15 to a4f8412 Compare October 19, 2020 07:46
@pawelpasterz
Copy link
Contributor

@bootstraponline bootstraponline force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch from 9de5636 to 3d2ab14 Compare October 19, 2020 10:27
@bootstraponline bootstraponline force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch from 191ea61 to 55cdaa0 Compare October 19, 2020 14:20
@bootstraponline bootstraponline force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch from df76968 to 7410843 Compare October 20, 2020 09:56
@jan-goral jan-goral force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch from f1140d5 to 15105a8 Compare October 23, 2020 13:14
@piotradamczyk5 piotradamczyk5 self-requested a review October 23, 2020 13:36
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 left a comment

Choose a reason for hiding this comment

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

Just address @Sloox 'es comments and good to go

And also please check tests on Windows 😄
Probably you could skip them for now with TODO comment about solving them in #1134

@bootstraponline bootstraponline force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch 3 times, most recently from dd4a268 to 97702ce Compare October 26, 2020 11:02
zuziaka and others added 19 commits October 26, 2020 11:57
- updated for using new test artefacts
- resolved conflicts
- updated code signing in EarlGreyExample project
- updated methods/test targets names
- added new iOS test project
- created ops.sh scripts to generate test artifacts from it
- adjusted fixture paths inside unit tests
- `findTestNamesForTarget` method

Co-authored-by: Jan Góral <[email protected]>
Small XCTestRun's methods refactor

Co-authored-by: Jan Góral <[email protected]>
@bootstraponline bootstraponline force-pushed the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch from 97702ce to 0f4021a Compare October 26, 2020 11:57
@jan-goral jan-goral merged commit 8a81c2d into master Oct 26, 2020
@jan-goral jan-goral deleted the #842-support-for-test-targets-flag-in-multiple-testbundles-xctest branch October 26, 2020 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests are included in XCTest entries that do not contain the test
6 participants