haf / expecto

A smooth testing lib for F#. APIs made for humans! Strong testing methodologies for everyone!
Apache License 2.0
668 stars 96 forks source link

[Feature Request] add automatic method-level discovery #261

Open 0x53A opened 6 years ago

0x53A commented 6 years ago

The biggest differentiator of Expecto is in data driven parametrized tests, where you can easily compose and generate tests without any of the awkward stuff of [x/n]unit. ([<MemberData("some-string")>] 😱 ).

For single test cases (aka xunit [<Fact>]), I prefer the method syntax, to be honest. This is also true (for me personally) in F#, but even more so in C#.

I propose to either add a new Attribute, or allow to place [<Tests>] on a method.

Now, please take the following design with a big sack of salt. I am not married to the exact details, but would like any way to simplify method-as-test.

Proposed Design:

1) use the existing attributes ([P/F]TestsAttribute) 2) automatically discover static generator methods which return Test or seq<Tes> in addition to properties. This would allow to take advantage of C# sequences. 3) automatically discover all static test methods with the attribute. Automatically wrap them in TestCases of class/method. Throw if they take any parameters, or return anything except unit/void/async/Task. 4) Do not automatically discover instance methods, but maybe add one or more helper functions to the lib. something like discoverTestMethodsInInstance : object -> Test.

public static class MyTests
{
    // automatically discovered
    [Tests]
    public static void Test()
    {
    }

    // automatically discovered
    [FTests]
    public static async Task TaskTest()
    {
        await Task.Delay(1);
    }

    // automatically discovered
    [PTests]
    public static IEnumerable<Test> TestGenerator()
    {
        yield return Runner.TestCase("1", () => { });
        yield return Runner.TestCase("2", () => { });
    }
}

public class InstanceTests
{
    string _databaseConnectionString;
    public InstanceTests(string databaseConnectionString)
    {
        _databaseConnectionString = databaseConnectionString
    }

    // NOT automatically discovered
    [Tests]
    public void InstanceTest()
    {
        // use _databaseConnectionString
    }

    [PTests]
    public static IEnumerable<Test> InstanceTestGenerator()
    {
        // use _databaseConnectionString
        yield return Runner.TestCase("1", () => { });
        yield return Runner.TestCase("2", () => { });
    }
}

class Program
{
    // this is initialized in Main, and then automatically discovered
    [Tests]
    public static Test InstanceTests { get; set; }

    public static void Main(string[] args)
    {
        // maybe I read the database connection strings from a configuration file, or ...
        InstanceTests =
            Runner.TestList("MyInstanceTests", new[] {
                Runner.DiscoverTestsInObject(new InstanceTests("some-database")),
                Runner.DiscoverTestsInObject(new InstanceTests("another-database"))
            });

        Runner.RunTestsInAssembly(Impl.ExpectoConfig.defaultConfig, args);
    }
}

Existing workaround:

Because expecto is so data-driven, I can do the same as proposed here from externally with a few helper methods.

Downside:

Currently, expecto is really simple, it just discovers all static properties, no magic. Maybe it would be better to keep it that way, and implement my proposed logic outside in helper functions.

AnthonyLloyd commented 6 years ago

Yes I think this could be helpful for people. Rust runs tests in parallel by default and does this.

0x53A commented 6 years ago

Thinking about this some more, I would prefer to keep discovery as-is, and just add the helper methods.

That is, RunTestsInAssembly will only discover static properties.

If you want to run static/instance methods, you will need to use the helper methods, and either assign it to a property prior to calling RunTestsInAssembly, or just directly use RunTestsWithArgs. I currently do the second.

Here are my current helper methods: https://gist.github.com/0x53A/7d751500f49592da00f4fff642161db5

Here is (simplified) how I use them:

var tests = new List<Test>();
tests.Add(Runner.TestList(nameof(MyTest), ExpectoEx.DiscoverInstanceTests(new MyTest(some-arguments))));
Runner.RunTestsWithArgs(Impl.ExpectoConfig.defaultConfig, testArgs.ToArray(), Runner.TestList("AllTests", tests));

If you agree, I would translate the C# class to F# and open a pull request?

All feedback welcome.

olivercoad commented 5 years ago

I like it, but definitely agree that it should just be the helper methods.

Could we also optionally pass in a config and then instead of throwing if they take parameters (or return bool), use testPropertyWithConfig? Would it be better to take a name and return a testList directly rather than IEnumerable<Test> to remove the need to manually call Runner.TestList (in the second line of your example use)?

I would be happy to help out with implementing this.

Ciantic commented 5 years ago

I was using xUnit with [<Fact>], but didn't like the output so I switched to Expecto.

It would be helpful if one wants to migrate from xUnit to have similar behavior, just placing an attribute above module level method. (I'm using F#)

Also I loose the navigation I'm used to, I use VSCode ionide and it's great symbol search with xUnit:

image

I think I can't live without my symbol search working for tests.

    [<Fact>]
    let ``Test addFlag option not found`` () = ...

    [<Fact>]
    let ``Test addValue option not found`` () = ...
Ciantic commented 5 years ago

I was so frustrated I came up my own quick an dirty solution, enjoy!

open Expecto
open System
open System.Reflection

[<Tests>]
let ``Test async version`` = async {
    do! Async.Sleep 1500
    Expect.equal true false "I fail!"
}

[<Tests>]
let ``Test discovered by the discoverer!`` () =
    Expect.equal true false "I fail!"

[<EntryPoint>]
let main argv =
    let myTests = 
        Assembly.GetExecutingAssembly().GetTypes() 
        |> Array.choose (fun t -> 
            // Create test case based on methodInfo.
            let createTestCase (mi: MethodInfo) =
                let a = Delegate.CreateDelegate(typedefof<Action>, mi) :?> Action
                testCase mi.Name a.Invoke

            // Create test case based on propertyInfo, assuming Async<unit>.
            let createTestCaseAsync (pi: PropertyInfo) =
                testCaseAsync pi.Name (pi.GetValue(null) :?> Async<unit>)

            // Get all members with TestsAttribute
            let testCases = t.GetMembers() |> Array.choose(fun (mi: MemberInfo) -> 
                mi.CustomAttributes 
                |> Seq.tryFind (fun attr -> attr.AttributeType = typeof<TestsAttribute>)
                |> Option.map (fun _ -> mi)) |> Array.choose (fun mi ->
                    match mi with 
                    | :? MethodInfo as mi -> Some (createTestCase mi)
                    | :? PropertyInfo as pi -> Some (createTestCaseAsync pi)
                    | _ -> None) |> List.ofArray

            if List.isEmpty testCases 
            then None 
            else Some (testList t.Name testCases)
        ) |> List.ofArray

    runTestsWithArgs defaultConfig argv (testList "All tests" myTests)

Can be improved substantially of course, but for the time being it works...