-
Notifications
You must be signed in to change notification settings - Fork 917
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
#1130 Initial swift package manager support #1193
Conversation
Since previous PR, Github Actions have been added, waiting for it to run I still need to cherry-pick my latest commit demoing integration using "path package" of KIF in a test project and ask review to @dostrander or @justinseanmartin Edit: first CI job is an epic fail:
run is at: https://github.com/kif-framework/KIF/pull/1193/checks?check_run_id=1392807500 |
Got Xcode 10.3 back... got issue on AutoCorrectTests::testSmartQuotesEnabled, and on AutoCorrectTests_ViewTestActor::testSmartQuotesEnabled (2 assertions per test => 4 failures), but not on the ones of Github Actions... 🙁
|
Same result with ci script :
|
tried again by running ci script in a ssh session... same errors
I suspect than user interaction is not enabled, so all tests are failing (maybe because of user not logged in on CI server) |
OK, tried Actions on another project, it is not "user interaction disabled" issue As Xcode run tests alphabetically, I move the [BackgroundTests testBackgroundApp] to the end by renaming its class to ZBackgroundTests:
EDIT: run is : https://github.com/kif-framework/KIF/pull/1193/checks?check_run_id=1396588868 |
Force pushed to trigger another build because first time iPad build was stopped because of fail in iPhone tests |
so there is an issue with |
nice, CI works for building SPM package, here is the command added to
|
As there are 3 projects built by CI ( |
Seems like a reasonable plan to me, but defer to @dostrander.
On Nov 14, 2020, at 8:27 AM, kenji21 <[email protected]> wrote:
As there are 3 projects built by CI (Testable Swift.xcodeproj, Testable.xcodeproj and Calculator.xcodeproj), to not duplicate code, @justinseanmartin<https://github.com/justinseanmartin> @dostrander<https://github.com/dostrander> what do you think of modifying one to integrate the framework using SPM (doing so will require Xcode 11 minimum for this project, thus adding Xcode 11 in the CI matrix)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1193 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABMVQJT3JQEJGMNI6BXMUSLSP2VVXANCNFSM4TTZJVAQ>.
|
@kenji21 Sounds good to me |
Tested on my mac using On
Found one issue: -(void) testPullToRefreshByAccessibilityIdentifierWithDuration
{
UITableView *tableView;
[tester waitForAccessibilityElement:NULL view:&tableView withIdentifier:@"Test Suite TableView" tappable:NO];
[tester tapViewWithAccessibilityLabel:@"Reset Refresh Control"];
[tester pullToRefreshViewWithAccessibilityIdentifier:@"Test Suite TableView" pullDownDuration:KIFPullToRefreshInAboutThreeSeconds];
[tester waitForViewWithAccessibilityLabel:@"Bingo!"];
[tester waitForAbsenceOfViewWithAccessibilityLabel:@"Bingo!"];
} makes refresh control label to have text "Refreshing...", and then - (IBAction)resetRefreshControl
{
self.refreshControl.attributedTitle = [[NSAttributedString alloc] initWithString:NSLocalizedString(@"Refreshing...", @"") attributes:nil];
} To fix this: -(void)pullToRefreshHandler
{
- self.refreshControl.attributedTitle = [[NSAttributedString alloc] initWithString:NSLocalizedString(@"Bingo!", @"") attributes:nil];
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(4.0 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+ self.refreshControl.attributedTitle = [[NSAttributedString alloc] initWithString:NSLocalizedString(@"Bingo!", @"") attributes:nil];
[self.refreshControl endRefreshing];
[self.tableView setContentOffset:CGPointZero animated:YES];
}); Found another one that could be the source of many issues: the photo selector is shown and not cancelled, so I moved SystemAlertTests to the end of execution Next actions should give a result like: |
Run 1410490218 gets better than expected:
As expected, System Alerts Tests are broken |
After fixing typing tests:
|
413d837
to
e10bdad
Compare
Smart dashes fixed:
let's continue by loading UIAutomation.framework correctly (note that UIAutomation.framework no longer exists within Xcode 12) |
Nice, Fixed AutocorrectTests, now 16 -> 12 fails, now time to fix System Alert Tests:
|
Ok, I was looking to |
As this PR was only intended to add SPM support, can we merge it, and treat the |
@kenji21 Can we add an early return if we detect that it is running xcode 11 for the system tests |
sure, i'll have to rebase my branch on master (pbxproj have many conflicts) |
=> #1156
=> #1196
=> Fixed by early return So Xcode 11 tests are OK, but carthage build using Xcode 12 is failing |
…n Xcode 12 (no UIAutomation framework)
Sorry, I haven't see it (confused by Xcode/iOS versions), it seems that's always the System Alert Tests that are also failing for iOS 11. :/
Just rebased on top of current master (this one was easier than the previous one, merging a pbxproj is hard) |
@kenji21 they also use the UIAutomation framework |
I am missing something, executed them on our CI after installing Xcode 11.6 and downloading simulator 11.4 (as it is in .travis.yml from current master) $ export DEVELOPER_DIR=/Applications/Xcode_11.6.app/Contents/Developer/
$ SIMULATOR="name=iPad Pro (12.9-inch) (2nd generation),OS=11.4"
$ ./scripts/ci.sh "${SIMULATOR}"
[...]
ZBackgroundTests
✓ testBackgroundApp (13.208 seconds)
ZSystemAlertTests
✓ testAuthorizingLocationServicesAndNotificationsScheduling (1.540 seconds)
✓ testAuthorizingPhotosAccess (1.538 seconds)
ZSystemAlertTests_ViewTestActor
✓ testAuthorizingLocationServices (1.539 seconds)
✓ testAuthorizingPhotosAccess (1.538 seconds)
Executed 275 tests, with 0 failures (0 unexpected) in 759.207 (768.921) seconds
[...] what surprises me is that Xcode 11.6 ship with iOS 13.6 see xcodereleases.com |
ok, it breaks in the
|
.github/workflows/build.yml
Outdated
@@ -12,7 +12,11 @@ jobs: | |||
matrix: | |||
run-config: | |||
- { xcode_version: '10.3', simulator: 'name=iPad (5th generation),OS=12.4' } | |||
- { xcode_version: '10.3', simulator: 'name=iPhone X,OS=12.4' } | |||
- { xcode_version: '10.3', simulator: 'name=iPhone 8,OS=12.4' } | |||
#- { xcode_version: '11.7', simulator: 'name=iPad (7th generation),OS=13.7' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in openium@4f286cf
KIF Tests/BackgroundTests.m
Outdated
@@ -8,11 +8,11 @@ | |||
|
|||
#import <KIF/KIF.h> | |||
|
|||
@interface BackgroundTests : KIFTestCase | |||
@interface ZBackgroundTests : KIFTestCase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename back to BackgroundTests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in openium@4f286cf
KIF Tests/BackgroundTests.m
Outdated
@@ -35,6 +35,9 @@ - (void)afterEach { | |||
} | |||
|
|||
- (void)testBackgroundApp { | |||
#if defined(__IPHONE_14_0) // Xcode 12, UIAutomation framework not available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we just do if (@available(iOS 14.0, *))
??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in openium@4f286cf
KIF Tests/SystemAlertTests.m
Outdated
@@ -8,11 +8,11 @@ | |||
|
|||
#import <KIF/KIF.h> | |||
|
|||
@interface SystemAlertTests : KIFTestCase | |||
@interface ZSystemAlertTests : KIFTestCase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename back to SystemAlertTests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in openium@4f286cf
KIF Tests/SystemAlertTests.m
Outdated
@@ -21,6 +21,9 @@ + (XCTestSuite *)defaultTestSuite | |||
if ([UIDevice.currentDevice.systemVersion compare:@"8.0" options:NSNumericSearch] < 0) { | |||
return nil; | |||
} | |||
#if defined(__IPHONE_13_0) // Xcode 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use @available instead of #if
s ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in openium@4f286cf
|
||
@interface SystemAlertTests_ViewTestActor : KIFTestCase | ||
@interface ZSystemAlertTests_ViewTestActor : KIFTestCase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change back to SystemAlertTests_ViewTestACtor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in openium@4f286cf
@@ -20,7 +20,7 @@ - (NSNumber *)AXInspectorEnabled:(id)specifier; | |||
@end | |||
|
|||
|
|||
static void * loadDylibForSimulator(NSString *path) | |||
void * loadDylibForSimulator(NSString *path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to use it from UIAutomationHelper.m, reverted in openium@4f286cf
Sources/KIF/UIAutomationHelper.m
Outdated
@@ -213,7 +214,12 @@ - (BOOL)_alertIsValidAndVisible:(UIAAlert *)alert; | |||
} | |||
|
|||
- (void)linkAutomationFramework { | |||
dlopen([@"/Developer/Library/PrivateFrameworks/UIAutomation.framework/UIAutomation" fileSystemRepresentation], RTLD_LOCAL); | |||
NSString *path = @"/Developer/Library/PrivateFrameworks/UIAutomation.framework/UIAutomation"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this help anything for uiautomation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tried to find why UIAutomation fails to load, see that loadDylibForSimulator
prepends IPHONE_SIMULATOR_ROOT to framework path, but it didn't helped, reverted in openium@4f286cf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this, i know a bunch of folks have been really excited for this
Can I ask for the plans to merge this? |
FYI, I've tested https://github.com/openium/KIF/tree/spm-support and found no problems so far in my projects. |
please use https://github.com/openium/KIF/tree/spm-support-freez as I may rewrite history (if squash needed for example) and force push on spm-support branch |
Thanks for the info 😀. |
@kenji21 This looks good to me once you are ready to merge let me know. Only reason i'm not currently is it still has the WIP title :) |
@kenji21 This has duplicate files, moved the files around without fixing the podspec, and not including anything in the IdentifierTests subspec. I need to revert this for now. If you could fix these issues that would be helpful. I am going to add a check to make sure that the podspec should validate which would have picked up these issues. |
#1130
As some project refers to openium/master and I don't want to force push to this branch, I created another branch to update my initial PR #1147 to current KIF master
Basically the same changes (removing precompiled headers (.pch) and moving files to be "more" SPM compliant)