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


closed
spascoe submitted on Wed 09 October 2019 #

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 on Thu 10 October 2019 # OWNER

@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:

  • When you edit out that line from Test.MicrosoftUnitTestDemo.pq you make its code incorrect. That's almost like adding a random comma somewhere - Power Query can not meaningfully parse such module and will show an error (Expression.Error: Name "UnitTesting.Returns123" not found). You can see that error if you try to reference the module directly: LibPQ("Tests.MicrosoftUnitTestDemo")
  • Test discovery mechanism relies on metadata being provided by modules that contain test suites:

    https://libpq.ml/Docs/UnitTesting/#test-suite

    Suites without the metadata will still be executable by test runners but will not be visible to discovery tools.

    Test suites have to be stored in separate LibPQ modules that contain metadata referring to the test suite version (value of LibPQ.TestSuite meta field).

  • If module can not be parsed, its metadata can not be parsed also - that makes the module invisible to UnitTest.Discover. Filtering happens here:
    https://github.com/sio/LibPQ/blob/7151fd51c5b86b0d4d036598d9c7945064cf8b1e/Modules/UnitTest.Discover.pq#L16-L30
    Modules without suite version are dropped.

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 referenced this issue in commit 8482ec7 on Thu 10 October 2019
sio commented on Thu 10 October 2019 # OWNER

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 on Thu 10 October 2019 #

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 on Thu 10 October 2019 # OWNER

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 closed this issue in commit dce231e on Fri 11 October 2019
sio commented on Fri 11 October 2019 # OWNER

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:

  • Start with your example above (with line 12 commented out in Tests/Tests.MicrosoftUnitTestDemo.pq)
  • Rename Tests/Tests.MicrosoftUnitTestDemo.pq to Tests/MicrosoftUnitTestDemo.pq. Now filename does not start with "test"
  • The fix from yesterday would've missed this failed test suite even though it does not violate any UnitTest docs
  • Today's fix will pick it up and show an error line in UnitTest.Discover() output

I've updated the changelog and merged changes into master