YoloDev / YoloDev.Expecto.TestSdk

27 stars 16 forks source link

Add `filter-test-case` to readExpectoConfig #131

Closed 64J0 closed 11 months ago

64J0 commented 11 months ago

Description:

When trying to filter the Expecto test that I'd like to run with filter-test-case, I noticed that it was indeed not working.

Example command:

dotnet test -- Expecto.filter-test-case="Some string"

Then, after digging the code, I noticed that the values are passed to the Expecto command by the function readExpectoConfig.

With this PR, I'm adding a new possible value that works with the filter-test-case.

Please let me know if you'd like something to change in order for this PR to be merged.

Related issue:

Must close https://github.com/YoloDev/YoloDev.Expecto.TestSdk/issues/130.

How to test:

# clone this repo
cd test/Sample.Test

# list all the available tests
dotnet test --list-tests

# filter the tests that are going to run
dotnet test -- Expecto.filter-test-case="test1"
dotnet test -- Expecto.filter-test-case="test4"

# assert that it ran only one test each time

image

64J0 commented 11 months ago

If this PR is merged, how long would I need to wait for it to be released to another package version? It is blocking me in the advance of a task.

Pinging @Alxandr and @gdziadkiewicz since you people looks like the most prolific human contributors of this project.

Alxandr commented 11 months ago

Please don't just ping people for no good reason.

That aside, this PR looks reasonable, but it would be much preferred to use the builtin filter mechanism for dotnet test. After all, this is a adapter for dotnet test. Also - I'm pretty sure filtering used to work (but I've not used this in years so I may be misremembering). Have you looked at the syntax for dotnet test --filter? https://learn.microsoft.com/en-us/dotnet/core/testing/selective-unit-tests?pivots=mstest

64J0 commented 11 months ago

@Alxandr I did try the --filter mechanism, and it did not work with my real project. I added some notes to this issue: https://github.com/YoloDev/YoloDev.Expecto.TestSdk/issues/130.

Indeed, it doesn't work even with the samples in this project.

64J0 commented 11 months ago

Can we have this PR merged, and the package updated while we don't have the --filter working, please?

Alxandr commented 11 months ago

That is not valid filter syntax. Did you read the specification?

64J0 commented 11 months ago

I did read @Alxandr, but was not able to use the --filter yet, as mentioned before with my screenshots. I tested the full name, but it did not work either, like:

dotnet test --filter list1.test1

It does still run all the tests.

Btw, this is what appears when I list the tests:

image

64J0 commented 11 months ago

Can you please show me an example of valid syntax for using the --filter in this project?

Alxandr commented 11 months ago

I re-read the documentation myself, and it is indeed the case that you should be able to simply write the fully qualified name as you have. I also debugged my way through the code, and it doesn't work as I remembered, so filtering does not work. But I made a PR that implements it using the proper mechanism, and not through special expecto properties. It's probably not perfect, and can definitely be improved, but for now it should work and improve IDE support.

64J0 commented 11 months ago

Ok, thanks for that @Alxandr. Hope this project thrives!

64J0 commented 11 months ago

Just to keep it recorded.

The --test filter apparently works "the same" as the Expecto flag filter-test-case. The default is to use the contains operator when filtering the tests (~ in the --filter case).

We have some weird necessities with test case names that have spaces.

To understand this behavior, you can:

  1. Tweak the test/Sample.Test/Test.fs file, using this configuration:
testCase "test 1" (fun () -> ())
testCase "test 12" (fun () -> ())
  1. Then, run the test using the filter:
dotnet test --filter "test 1"

Notice that it's going to run both "test 1" and "test 12" due to the contains operator.

If we wanted to run with the exact match operator, we would need to tweak the filter string, although I was not able to make it work yet.

Some filters that I tested:

dotnet test --filter "test&1"
dotnet test --filter "Name=test&1"
dotnet test --filter "Name=test&Name=1"
dotnet test --filter "(Name=test)&(Name=1)"
dotnet test --filter "Name=(test&Name=1)"

As mentioned before, it worked the same when testing with the filter-test-case locally.

Alxandr commented 11 months ago

While looking into this, I read somewhere about an issue with spaces (don't remember if it was expecto or testsdk issue though). I recommend debugging and trying to figure out why it doesn't work. If it's a bug, create an issue about it. I added a environment variable check, so if you set DEBUG_EXPECTO_TESTSDK, that will launch the debugger (on windows) - or hang the process printing the process number on other systems. The function used for matching can be found here: https://github.com/YoloDev/YoloDev.Expecto.TestSdk/blob/main/src/YoloDev.Expecto.TestSdk/settings.fs#L112

64J0 commented 11 months ago

Great, thanks for sharing!