sio / LibPQ

Detach your M code from workbooks to reuse it! Import modules from local or web storage (unlimited number of sources)
Apache License 2.0
76 stars 21 forks source link

Unit Testing - Tests with coding errors disappear from test run results #18

Closed spascoe closed 4 years ago

spascoe commented 4 years ago

When a unit test file has an error in it, the entire file's tests disappear from the UnitTest Results. To Reproduce do the following:

  1. With LibPQ configured to run the LibPQ Unit Tests, Run UnitTest.Discover()
  2. Notice that the Test.MicrosoftUnitTestDemo tests are listed.
  3. Edit Tests\Test.MicrosoftUnitTestDemo.pq and comment out line 12 (UnitTesting.Returns123...)
  4. Save and refresh the UnitTest.Discover()

The Test.MicrosoftUnitTestDemo tests are no longer listed. It would be more desirable to list the file as a single row with an error. I've looked through the code, and tried a number of things, without success, so thought it was worthy of an "Issue".

I'm glad to help with resolving, if someone could give me some pointers.

sio commented 4 years ago

@spascoe, thank you for taking time to write a comprehensive report!

I've tested this and can replicate the behavior you've described. Both fact based unit tests and native LibPQ unit tests are affected.

Here is why it happens:

Even though everything works exactly as documented, I agree it is not the best possible behavior.

I think we can propagate errors for modules that look like test suites with a fake "test runner". The question is how do we decide that a module looks like a test suite? What's your opinion?

I'll start implementing the workaround with some placeholder logic for now.

sio commented 4 years ago

Here is what I've come up with: 8482ec7392c4e78e8feacc136605c133170f6f8e

If a module errors out on import and its name starts with "test" (case insensitive), all import errors are propagated to UnitTest.Discover output.

spascoe commented 4 years ago

I tested your solution, and got exactly the result that I was expecting to see. That is an awesome solution. I used the following script to get a nice report:

let
    UnitTest.Discover = LibPQ("UnitTest.Discover"),
    Tests = UnitTest.Discover(),
    JustDetails = Table.RemoveColumns(Tests,{"Status", "Count"}),
    DetailedResults = Table.ExpandTableColumn(
        JustDetails, "Details", 
        {"Suite", "Test", "Result", "Status", "Description"}, 
        {"Details.Suite", "Details.Test", "Details.Result", "Details.Status", "Details.Description"})
in
    DetailedResults
sio commented 4 years ago

Thanks :) Your preferred report layout is already supported by UnitTest.Discover - you can run LibPQ("UnitTest.Discover")(false) to get it. See docstring for that module: https://github.com/sio/LibPQ/blob/8482ec7392c4e78e8feacc136605c133170f6f8e/Modules/UnitTest.Discover.pq#L1-L10

I'll leave this in a feature branch overnight to see if some bright idea pops up in my head.

I don't like relying on the module name much, because it's not really required to start with "test" by any other part of UnitTest framework. Inspecting raw source code for the module with import error seems to be excessively difficult. The loader does not expose it in any reusable way, and replicating a lot of the loader logic or modifying the loader itself are both worse in my opinion than current filename solution.

PS: I am glad that you find this project interesting! Would you mind leaving some feedback on how/where you use it? Please reply to this issue

sio commented 4 years ago

I've found a clean way to propagate custom metadata along with module evaluation errors, so now test suites can be correctly discovered in modules with arbitrary names. You'll need to update LibPQ query in your workbooks to make use of enhanced test discovery.

Demo of improved behavior:

I've updated the changelog and merged changes into master