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

Public Filter/FilterFlags replacement #1145

Open
1 task done
stonko1994 opened this issue Apr 20, 2022 · 1 comment
Open
1 task done

Public Filter/FilterFlags replacement #1145

stonko1994 opened this issue Apr 20, 2022 · 1 comment

Comments

@stonko1994
Copy link
Contributor

  • I have read CONTRIBUTING and have done my best to follow them.

What did you do?

I am currently trying to update to version v5.0.0 as it contains a fix when using Xcode 13.3.

However, version 5.0.0 has a breaking change I currently (heavily) rely on. Currently, I use the Filter.pending for following use-cases: all at runtime

  • Skipping certain tests within a test class based on the iOS version
  • Skipping certain tests within a test class if they are executed on a simulator or a physical device
  • Skipping certain tests within a test class based on the architecture (mainly arm because of throwAssertion not working with Rosetta  Nimble#961)

I do understand why they were removed from public API and that they shouldn't have been there in first place.
Though is there a plan for a replacement API or a new feature for this kind of use-cases?

Right now I can work around this and it doesn't block me from updating but it would be nice to have a feature like this build in Quick.


This issue is very similar to #800 but as it was closed recently and wasn't very active and I don't comply with the API and approach used in the original post I decided to open a new one. The proposed API there seems a bit fragile and not Swift-like to work with 'tags' in the test name itself.

As proposed in #800, exclusion-filters as in Rspec could be interesting. As I understand this feature is basically there internally but not exposed to the public.

I would be happy to contribute if possible and once we settled on a good API design.

Environment

List the software versions you're using:

  • Quick: 5.0.1
  • Nimble: 9.2.0
@stonko1994
Copy link
Contributor Author

stonko1994 commented Apr 20, 2022

I was playing around with this a bit and build a (maybe overcomplicated) POC.


Changes

public func it(
    _ description: String,
    tags: [Tag: Bool] = [:],
    file: FileString = #file,
    line: UInt = #line,
    closure: @escaping () throws -> Void
) {
    World.sharedWorld.it(description, tags: tags, file: file, line: line, closure: closure)
}

This re-adds a dictionary property to the describe, context and it methods. In my first version I called this tags but I think I would prefer calling them flags. (The internal flags property might need another name in this case)

The Tag type that I used as a key for the dictionary would be an enum with only one case:

public enum Tag {
    case string(String)
}

Again, my preferred name would be Flag.

This enum conforms to ExpressibleByStringLiteral, CustomStringConvertible, Hashable and Equatable. Under the hood, this makes it just a String but with the enum it would allow a quite nice API usage.

When it comes to filtering for the tags I created a convenience extension for QCKConfiguration that enables a simple configuration.

Naming TBD. Took the Rspec naming here.

public extension QCKConfiguration {
    func filterRunExclude(_ tag: Tag, _ excludedValue: Bool) {
        exclude { example in
            // ...
        }
    }
}

In order to pass the tags along, I also added them to the Example class.

public let tags: [Tag: Bool]

Usage

The advantage of the Tag enum would be that everyone could define their own 'cases'.

extension Tag {
    static var isFlaky: Self { "isFlaky" }
}

As the enum just uses a String something like the above would be required but the string could be anything unique in this case.
I abstracted that way with some magic PropertyWrapper just creating a UUID for every case 😅 (I'm not happy with this)

With the PropertyWrapper it would look like this:

extension Tag {
    @LazyTag
    static var isFlaky
}

In order to use the tag now on the test itself, you can use the dot syntax:

it("...", tags: [.isFlaky: true]) {

When it comes to filtering for the tag now, the new method on the QCKConfiguration can be called within a QuickConfiguration subclass: (See Rspec usage here)

class TagConfiguration: QuickConfiguration {
    override class func configure(_ configuration: QCKConfiguration) {
        configuration.filterRunExclude(.isFlaky, isRunningOnCi)
        configuration.filterRunExclude(.foo, true)
        configuration.filterRunExclude(.bar, false)
    }
}

Maybe the enum is overengineered. As an alternative plain string handling with constants would work as well. The enum would rule out string handling and potential errors in this case though.

Please consider that this is a POC


Open questions:

  • Why is the _ExampleBase prefixed with an _? Should it be considered an internal class? If yes, how to handle the Example class and methods that are using it?

Feedback welcome 🙂

@younata younata added this to the v6.0.0 milestone Jun 6, 2022
@younata younata modified the milestones: v6.0.0, v7.0.0 Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants