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.

Sign up for GitHub

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

Jump to bottom

Initial Support for .xctestrun File Format Version 2 #559

Merged
merged 46 commits into from
Feb 11, 2025
Merged

Initial Support for .xctestrun File Format Version 2 #559

merged 46 commits into from
Feb 11, 2025

Conversation

bahrimootaz
Copy link
Contributor

@bahrimootaz bahrimootaz commented Feb 7, 2025 •
edited
Loading

Description :
Enable the execution of XCTest cases using .xctestrun configuration files in version 2 format, expanding compatibility and aligning with latest Xcode standards.

Context :
Currently, go-ios supports .xctestrun files with format version 1. However, Xcode has introduced version 2 of the .xctestrun file format, which brings structural changes and enhancements. To maintain compatibility with modern XCTest setups and improve usability for customers, we need to support this newer format.

Reference: new structure of the .xctestrun is documented here

Testing :
In this MR we test did test the following scenarios :

  • contains only a single XCTest 🟢
  • contains multiple XCTest targeting the same app 🟢
  • contains multiple XCTest targeting different apps 🟢

bahrimootaz and others added 19 commits February 5, 2025 17:03
@bahrimootaz
Initial commit - Support of xctestrun v2
5039b87
@bahrimootaz
PoC with reset listener
ecc2a59
@bahrimootaz
PoC with reset listener
c556747
@bahrimootaz
Run both XCTest And XCUITEST
cb937d8
@bahrimootaz
Run both XCTest And XCUITEST
cce1e4a
@bahrimootaz
use the UITargetAppPath to grab the bundle id of the target app
8280396
@bahrimootaz
remove prints
f107fa9
@si-net
Polish code for iterating over configs
f13cc2e
@bahrimootaz
Parse xctstrun v2 and trigger all test configuration
d2be18c
@si-net
Remove obsolete fies
aa34bb6
@si-net
Remvoe more obsolete files
df26dd9
@si-net
Obsolete changes
cce2c3a
@si-net
Merge branch 'main' into polish-code
47c8d13
@bahrimootaz
Merge pull request #2 from bahrimootaz/polish-code
7062b58 
Polish code for iterating over configs
@bahrimootaz
Allow UI tests to run without a target app. For example, WebDriverAge…
7654fed 
…nt doesn't require a BundleId.
@bahrimootaz
Remove unnecessary test
d94ca21
@bahrimootaz
Revert "Remove unnecessary test"
a9d80bf 
This reverts commit d94ca21.
@bahrimootaz
Enforce stricter checks for XCTestrun format version 2: Added restric…
a4071c3 
…tions on missing or multiple TestConfigurations. Only one TestConfigurations is expected
@bahrimootaz
Avoid returning a empty string when we can't locate the bundleId
fc2aa6f
si-net
si-net suggested changes Feb 10, 2025
View reviewed changes
@bahrimootaz bahrimootaz changed the title Full Support for .xctestrun File Format Version 2 Partial Support for .xctestrun File Format Version 2 Feb 10, 2025
si-net and others added 6 commits February 10, 2025 17:39
@si-net
Reset Testlistener at correct place
8647066
@bahrimootaz
Merge pull request #6 from bahrimootaz/fix-testlistener-signalling
dd91b59 
Reset Testlistener at correct place
@si-net
Refactor xctestrunutils test
7dfa02c
@bahrimootaz
Merge pull request #7 from bahrimootaz/refactor-tests
ff3e730 
Refactor xctestrunutils test
@si-net
Refactor tests to be DAMP instead of DRY
2698097 
The helper method to load the different test targets, required the reader to make a lot of mental
jumps to understand which test target was actually used in a test. Tests are not production code and should
be as simple to follow along as possible. Now, when a reader wants to know which target is used, they know that
they should look at the file that is located at /testdata/format_version2... and they know that they need to look at the
first or second entry of the TestConfiguration array.
This is important when the reader wants to understand the relation between the xctestrun file and the code.
@si-net
More refactors
9a3d9ef
@bahrimootaz bahrimootaz changed the title Partial Support for .xctestrun File Format Version 2 Initial Support for .xctestrun File Format Version 2 Feb 10, 2025
@isevendays-saucelabs
Copy link

Looks good to me. I think PR includes many tests and overall has a good structure.

dmissmann
dmissmann requested changes Feb 11, 2025
View reviewed changes
ios/testmanagerd/xctestrunutils_test.go Outdated
// Assert: Validate the returned TestConfig
assert.NoError(t, err, "Error converting to TestConfig")

return testConfig, mockDevice, mockListener
Copy link
Collaborator

Choose a reason for hiding this comment

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

mockDevice and mockListener are always ignored, we can remove them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ios/testmanagerd/xctestrunutils_test.go
@@ -0,0 +1,363 @@
package testmanagerd
Copy link
Collaborator

Choose a reason for hiding this comment

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

The number of tests can be reduces significantly. There are nine tests using parseXCTestRunFileVersion1, and that can simply be one. Initialize the schemeData struct to what you expect and just assert against that. The test output should show you the differences in a easy to read format which also makes the assertion messages like "TestHostBundleIdentifier mismatch" obsolete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we did restructured the testing file, can you please check again.

@si-net
Copy link
Contributor

si-net commented Feb 11, 2025 •
edited
Loading

We discovered another issue with how we accumulate the test suites, we will have to revisit this today. @dmissmann .

  1. The TestListener accumulates all test suites in a list.
  2. In the loop that executes the test targets we also accumulate the test suites.
    --> this results in us saving all test suites twice.

We have two options:

  1. Reset more state of the listener after each execution, inside the reset() method
  2. Dont aggregate the suites in the loop, but let the listener aggregate the state. That would require us to also change some other types in the listener, suchas the err (would need to become an array).

The reset of the TestListener seems to become almost the same, as constructing a new TestListener by now, we could think about just constructing a new one for each target.

@dmissmann dmissmann merged commit aa0c039 into danielpaulus:main Feb 11, 2025
2 checks passed
@bahrimootaz bahrimootaz mentioned this pull request Feb 11, 2025
Merged
dmissmann pushed a commit that referenced this pull request Feb 12, 2025
@bahrimootaz
Enable Full Support for Multiple XCTest/XCUITest Cases configured in …
35e364b 
…xctestrun File (#563)

This PR extends the support for xctestrun files with format version 2, building on the initial implementation merged in #559.

With this update, we enable full support for XCUITests by utilizing the parsed test configurations from an xctestrun file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmissmann dmissmann dmissmann approved these changes

@si-net si-net Awaiting requested review from si-net

@rillgen-saucelabs rillgen-saucelabs Awaiting requested review from rillgen-saucelabs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bahrimootaz @isevendays-saucelabs @si-net @dmissmann @rillgen-saucelabs