-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add XCTest extension for comparing Color objects #889
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #889 +/- ##
=======================================
Coverage 92.92% 92.92%
=======================================
Files 100 100
Lines 3448 3448
=======================================
Hits 3204 3204
Misses 244 244
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Generated by 🚫 Danger |
Just reading over your comment now... Since this is our first test extension, I'd probably vote for something cheap and cheerful:
This is something we encourage with our extensions in general, right? Seems like a logical direction here, and sidesteps the build issues outlined above. If adoption is high and users are clamoring for a better solution, we can also investigate more complicated distribution mechanisms (Cocoapods, Carthage, SPM, etc) |
@guykogus This still WIP? Let us know when it's ready |
I was waiting to hear feedback from yourself and @omaralbeik. It's ready based on the current limitations. |
BTW I'm looking forward to Xcode 12 going live so that we can have a |
Ah sorry, I saw the WIP so was actually waiting too ... haha
Uhum, I've never run into this, but my guess is that we have to tell the subspec that it does need the |
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.
Overall looks good to me! Just some minor points
* 'master' of https://github.com/SwifterSwift/SwifterSwift: (79 commits) Helper functions for scrolling to the edges or by page of a scroll view (SwifterSwift#888) Add XCTest extension for comparing Color objects (SwifterSwift#889) Fix swiftlint error (SwifterSwift#894) Bump kramdown from 2.1.0 to 2.3.0 (SwifterSwift#891) Added note for UIView.addShadow() (SwifterSwift#890) loadFromNib(withClass) (SwifterSwift#885) constraint finders (widthConstraint, heightConstraint, leadingConstraint, trailingConstraint, topConstraint, bottomConstraint) (SwifterSwift#886) added String.regexEscaped (SwifterSwift#883) Make √ generic for FloatingPoint values (SwifterSwift#880) Update UITextField extensions (SwifterSwift#878) Added `masksToBounds` (IBInspectable) extension (SwifterSwift#877) Reinstate debounce function (SwifterSwift#869) `RandomAccessCollection`/`Collection` generalisations (SwifterSwift#863) Added HKActivitySummary extensions (SwifterSwift#875) Added new method for easily instantiating a view controller from a storyboard (SwifterSwift#860) Clean up code (SwifterSwift#864) fixed "".truncated crashed (SwifterSwift#866) Deprecated map(by:), compactMap(by:), filter(by:) (SwifterSwift#862) Overloaded 'contains' operator for string regex matching (SwifterSwift#858) Added convenient wrapper to asyncAfter (SwifterSwift#859) ... # Conflicts: # Sources/SwifterSwift/AppKit/NSColorExtensions.swift # Sources/SwifterSwift/AppKit/NSImageExtensions.swift # Sources/SwifterSwift/AppKit/NSViewExtensions.swift # Sources/SwifterSwift/CoreGraphics/CGColorExtensions.swift # Sources/SwifterSwift/CoreGraphics/CGFloatExtensions.swift # Sources/SwifterSwift/CoreGraphics/CGPointExtensions.swift # Sources/SwifterSwift/CoreGraphics/CGSizeExtensions.swift # Sources/SwifterSwift/Foundation/NSAttributedStringExtensions.swift # Sources/SwifterSwift/SwiftStdlib/StringExtensions.swift # SwifterSwift.xcodeproj/project.pbxproj # Tests/AppKitTests/NSColorExtensionsTests.swift # Tests/SwiftStdlibTests/StringExtensionsTests.swift
🚀
As this is our first XCTest extension it's a bit tricky to work out where everything goes, so this is mainly starting as a discussion.
Checklist
@available
if not.Issues
Targets
Source files that include
XCTest
can only be added to test targets.This means that they can't be included in the frameworks in the Xcode project, which means that they can't be included in Carthage.
If a non-test target makes any reference to
XCTest
, even if it's protected byit will throw a build-time error saying Failed to load module 'XCTest', so it will probably cause projects to fail building if we add it via CocoaPods and, probably, SPM.
Location
Where do the source files belong? If we place them in
Sources/SwifterSwift/XCTest
then they will be included in the Swift package, unless we add that folder to theexcludes
list, which is easy enough.However, if we add it to
Tests/XCTest
then people may miss those extensions because they never open that folder. Maybe not 🤷♂️Imports
In order to use
Color
in this example I need to call@testable import SwifterSwift
, which may cause problems if we do find a way to make these extensions part of the framework. A solution would be to copy thetypealias
es.Regardless, it's not really an issue unless we can actually include it in the library along with the other files anyway.
New project
An alternative is to actually have a separate project, let's say called SwifterTests, which has SwifterSwift as a dependency. It would allow anyone who's mad enough to include the entire project as a dependency to include SwifterSwift for their project and SwifterTests to their test targets. E.g.
Obviously there are a bunch of issues here, the main one being visibility, but I think it's the cleanest option.