search

boost-ext / di

C++14 Dependency Injection Library
https://boost-ext.github.io/di
1.16k stars 140 forks source link

Rename test "ft_di_errors" to "ft_di_errors_may_take_a_minute_or_two" #408

Closed jamespharvey20 closed 4 years ago

jamespharvey20 commented 5 years ago

All of the other tests complete in fractions of seconds. This one can take a minute or two, so let the user know this so they don't thing it hung.

(I'm assuming it's normal for it to take a while, and not a bug.)

jamespharvey20 commented 5 years ago

Some travis-ci tests originally failed. Now also updates test/Jamfile.v2, expected to prevent the failures.

krzysztof-jusiak commented 5 years ago

I like the intent but I'm not sure whether renaming the file is the best solution here :thinking: Maybe we could add some log in the ft_errors or something, not sure?

jamespharvey20 commented 5 years ago

I don't know if it's the best solution, either. It's absolutely very hacky! I looked more, and wasn't able to find a better solution.

I'm also confused. If I run ctest, it shows:

 61/157 Test  #61: test.ft_di_errors ......................   Passed   80.17 sec

Yet, time test/test.ft_di_errors only takes 7.431s. Maybe I'm missing something, but I'm not seeing a reason for the 10x discrepancy. This is within the same build directory, without a rebuild. The 80 seconds through ctest is on a 2.9GHz Xeon with a Samsung NVMe 960 EVO, and 64GB RAM. I even removed my build directory, ran cmake, make, then the test without having ran cmake just in case there was some type of caching somehow not used when it is ran through cmake, but no difference.

Printing to stdout or stderr from within the test of course is hidden by default by ctest, and I don't think a solution like only showing the message if ctest had a verbose argument would be a proper fix.

I added a label within test/CMakeLists.txt:

set_tests_properties(test.ft_di_errors PROPERTIES LABELS "This may take a minute or two")

But, the label is unfortunately only shown at the end as a summary, not when it's actually being ran.

I asked in #irc on freenode, but got no response.

I asked a question on stackoverflow, but at least so far got no responses.

Since add_test takes a name and a command, it would be possible to not rename the file and built executable, but just the name displayed when running ctest. That would require modifying the cmake function test, or not calling it but doing everything manually for this one test.

krzysztof-jusiak commented 4 years ago

Closing it for now to clean up the issues; @jamespharvey20 - we can revisit as we go to find the best solution :+1: