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

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

Closed
spascoe opened this issue Oct 9, 2019 · 5 comments
Closed

Comments

@spascoe
Copy link

spascoe commented Oct 9, 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
Copy link
Owner

sio commented Oct 10, 2019

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

    SuitesFilter = Table.SelectRows(
    Table.AddColumn(
    Record.ToTable(Library),
    "SuiteVersion",
    each
    try Text.From(
    Record.Field(
    Value.Metadata([Value]),
    Config[Suite.MetaField]
    )
    ) otherwise
    null
    ),
    each [SuiteVersion] <> null
    ),

    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 added a commit that referenced this issue Oct 10, 2019
Previously if a module containing the test suite could not be imported
without errors such test suite would be invisible to discovery tools.

Now import errors are propagated to discovery results if a module looks
like a test suite.

See: #18
@sio
Copy link
Owner

sio commented Oct 10, 2019

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

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
Copy link
Author

spascoe commented Oct 10, 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
Copy link
Owner

sio commented Oct 10, 2019

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:

/**
Discover and run tests from all local sources.
Represent results as a table either grouped by status (compact_output = true)
or detailed with one row per each test (compact_output = false).
**/
(optional compact_output as nullable logical) =>
let
/* Default behavior is not yet decided upon and may be changed in future */
Compact = if compact_output <> null then compact_output else true,

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 sio closed this as completed in dce231e Oct 11, 2019
@sio
Copy link
Owner

sio commented Oct 11, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants