Regression on running CTest with v1.18.41 update

ebouteillon commented 4 weeks ago

Brief Issue Summary

When I click on the status bar button "CMake: run CTest tests":

With version v1.18.41 update, unit tests fail to run.
With version v1.17.17 update, unit tests run.

Context : I use WSL2 to run a CentOS7 linux environment. My project has 2400+ unit tests written with GTest. CMake detects them all with the built-in gtest_discover_tests(target) command. And thus all tests are visible in the "Testing" pane of VSCode.

When I click the status bar "CMake: run Ctest tests", I observe a change in the command line generated and executed by the VSCode extension:

v1.17.17 executes this (short) command:

[proc] Executing command: /opt/cmake/bin/cmake --build /home/axis/workspace/map-core/build --config Debug --target all -j 4 --

v1.18.41 executes a 161k+ characters long command (truncated to keep thing readable):

[proc] Executing command: /opt/cmake/bin/ctest -j4 -C Debug -T test --output-on-failure -R "^pre_ctest$|^post_ctest$|^HexaConvertorTest\.testConvertStringToHexa$|^HexaConvertorTest\.testConvertStringToHexaToString$|^HexaConvertorTest\.testConvertHexaToByte$|^HexaConvertorTest\.testConvertHexaToString$|^SerializerTest\.testConstructor$|^SerializerTest\.testSimpleSerialize$|^SerializerTest\.testSimpleDeserialize$|^SerializerTest\.testConstructJson$|^SerializerTest\. 
 ...snip... |^ResolverTest\.getSuccess$"
[ctest] CTest finished with return code -1

1/ In "Testing" pane, I see the timer running as if VSCode is waiting the end of tests execution... that failed immediately.

2/ The new extensions version generates a command which lists all the 2400+ tests. The command is 161k+ character long. I tried to run it in a shell but it fails immediately with bash: opt/cmake/bin/ctest: Argument list too long. Searching the web I found that you can increase size using ulimit but can only be up to 65K. So still not enough to fix the issue.

Can you fix this generated command please?

CMake Tools Diagnostics

{
  "os": "linux",
  "vscodeVersion": "1.89.1",
  "cmtVersion": "1.18.41",
  "configurations": [
    {
      "folder": "/home/axis/workspace/map-core",
      "cmakeVersion": "3.26.4",
      "configured": true,
      "generator": "Unix Makefiles",
      "usesPresets": false,
      "compilers": {
        "C": "/opt/rh/devtoolset-12/root/usr/bin/gcc",
        "CXX": "/opt/rh/devtoolset-12/root/usr/bin/g++"
      }
    }
  ],
  "cpptoolsIntegration": {
    "isReady": true,
    "hasCodeModel": true,
    "activeBuildType": "Debug",
    "buildTypesSeen": [
      "Debug"
    ],
    "requests": [],
    "responses": [],
    "partialMatches": [],
    "targetCount": 97,
    "executablesCount": 32,
    "librariesCount": 36,
    "targets": []
  },
  "settings": [
    {
      "communicationMode": "automatic",
      "useCMakePresets": "auto",
      "configureOnOpen": true
    }
  ]
}

Debug Log

[main] Building folder: map-core 
[build] Starting build
[proc] Executing command: /opt/cmake/bin/cmake --build /home/axis/workspace/map-core/build --config Debug --target all -j 4 --
[build] [  1%] Built target axmapserializer_object
[build] [  3%] Built target axmapcommon_object
[build] [  5%] Built target axmapcommon_static_object
[build] [  6%] Built target axmaptool_object
[build] [  7%] Built target axmaptool_static_object
[build] [  7%] Built target axmapserializer_static_object
[build] [ 15%] Built target axmapsql_object
[build] [ 16%] Built target axmappluginmanager_object
[build] [ 18%] Built target axepas-map_object
[build] [ 25%] Built target axmapsql_static_object
[build] [ 25%] Built target axmappluginmanager_static_object
[build] [ 26%] Built target axsettlementmap_object
[build] [ 34%] Built target axmap_testu_framework_object
[build] [ 36%] Built target axepas-map_static_object
[build] [ 37%] Built target axsettlementmap_object_static
[build] [ 37%] Built target axmoustacheprocessor
[build] [ 46%] Built target axmap_testu_framework_static_object
[build] [ 46%] Built target axmapserializer
[build] [ 46%] Built target axepas-map_static
[build] [ 46%] Built target axmapcommon_static
[build] [ 46%] Built target moustacheprocessortool
[build] [ 46%] Built target axmaptool_static
[build] [ 46%] Built target axmaptool
[build] [ 46%] Built target axmapserializer_static
[build] [ 46%] Built target axmappluginmanager_static
[build] [ 46%] Built target axmap_testu_framework_static
[build] [ 46%] Built target axmapsql
[build] [ 46%] Built target axmapcommon
[build] [ 46%] Built target axepas-map
[build] [ 46%] Built target axmapsql_static
[build] [ 47%] Built target libaxserializermaptestu
[build] [ 47%] Built target axsettlementmap
[build] [ 47%] Built target axmappluginmanager
[build] [ 47%] Built target axsettlementmap_static
[build] [ 47%] Built target receipt_formatter
[build] [ 48%] Built target axcfgcpaww
[build] [ 49%] Built target cleancpaww
[build] [ 49%] Built target clocpaww
[build] [ 50%] Built target telcpaww
[build] [ 51%] Built target advcpaww
[build] [ 51%] Built target advreinjectorcpaww
[build] [ 52%] Built target advsynccpaww
[build] [ 56%] Built target tpvcpaww
[build] [ 57%] Built target advroutercpaww
[build] [ 57%] Built target axmap_testu_framework
[build] [ 57%] Built target libaxpluginmanagermaptestu
[build] [ 57%] Built target cpa_plugin_test1_v1.0.0
[build] [ 59%] Built target advroutertestu
[build] [ 60%] Built target cpa_plugin_test2_v1.0.1
[build] [ 60%] Built target db_trx_generator
[build] [ 61%] Built target settlement_sample
[build] [ 61%] Built target common_toolstestu
[build] [ 62%] Built target moustacheprocessortestu
[build] [ 64%] Built target settlementmaptestu
[build] [ 65%] Built target epasmaptestu
[build] [ 65%] Built target microservicetestu
[build] [ 66%] Built target commonstestu
[build] [ 68%] Built target axcfgcpatestu
[build] [ 75%] Built target dbmanagerstestu
[build] [ 76%] Built target toolstestu
[build] 192 INFO: PyInstaller: 6.2.0
[build] 193 INFO: Python: 3.8.18
[build] 194 INFO: Platform: Linux-5.15.146.1-microsoft-standard-WSL2-x86_64-with-glibc2.2.5
[build] 194 INFO: wrote /home/axis/workspace/map-core/build/source/apx/python3/apxcpaww.spec
[build] 197 INFO: Extending PYTHONPATH with paths
[build] ['/home/axis/workspace/map-core/source/apx/python3',
[build]  '/opt/rh/rh-python38/root/usr/lib/python3.8/site-packages']
[build] [ 80%] Built target teltestu
[build] [ 88%] Built target tpvtestu
[build] [ 96%] Built target tpv_gtestu
[build] [ 96%] Built target cleantestu
[build] [ 97%] Built target clotestu
[build] [ 98%] Built target advreinjectortestu
[build] [ 98%] Built target libaxserializermapgeneratedtestu
[build] [100%] Built target advtestu
[build] 425 INFO: checking Analysis
[build] 427 INFO: checking PYZ
[build] 468 INFO: checking PKG
[build] 469 INFO: Bootloader /opt/rh/rh-python38/root/usr/local/lib/python3.8/site-packages/PyInstaller/bootloader/Linux-64bit-intel/run
[build] 469 INFO: checking EXE
[build] [100%] Built target apx_compile
[driver] Build completed: 00:00:00.758
[build] Build finished with exit code 0
[proc] Executing command: /opt/cmake/bin/ctest -j4 -C Debug -T test --output-on-failure -R "^pre_ctest$|^post_ctest$|^HexaConvertorTest\.testConvertStringToHexa$|^HexaConvertorTest\.testConvertStringToHexaToString$|
...snip...|^ResolverTest\.getSuccess$"
[ctest] CTest finished with return code -1

Additional Information

No response

nickassink commented 3 weeks ago

Hello, I had the same issue last Friday. I managed to fix it by disabling the Allow Parallel Jobs checkbox. This configuration option never used to work for me, now it works but the command is to big.

This will at least allow you to continue even if it is far from perfect.

ebouteillon commented 3 weeks ago

I have no issue with "CTest parallel jobs" with project tests. For the moment, I downgraded to previous VSCode plugin version v1.17.17 till it is fixed to have something functional.

gcampbell-msft commented 3 weeks ago

@ebouteillon Does the workaround that @nickassink mentioned work for you?

To avoid the very long command string, you should disable the "Allow parallel jobs" setting. This will avoid the very long command. However, to understand your scenario, what exactly are you hoping works? Are you hoping to be able to run all of your tests, in parallel, with the test explorer? While you use the workaround or downgrade, I'm happy to investigate something that might make you scenario still possible.

gcampbell-msft commented 3 weeks ago

@ebouteillon I investigated and am curious if a quick fix I implemented here would solve your issues: cmake-tools.zip

Could you download the above zip file, modify the extension to .vsix and then install it and let me know if it works for you? If it does, we can PR this and get it into pre-release upon merging.

ebouteillon commented 3 weeks ago

Hi Garrett,

I forgot to mention my projects use CTest's FIXTURES_SETUP and FIXTURES_CLEANUP. It is used for instance to setup/cleanup a database for the tests. So when I enable "CMake > CTest: Allow Parallel Jobs" then CTest organizes the tests to run fixtures only once before/after all the tests requiring these fixtures. If this option is disabled, then fixtures are run before/after every tests requiring them, which in the end is a lot slower. So my ideal use case is to have this option "Allow Parallel Jobs" enabled.

Here are my tests:

v1.18.41 => issue with the command line too long
v1.19.2 (pre-release) => issue with the command line too long
[provided vsix] v1.13.0 (pre-release) => issue with the command line too long

Unfortunately it is not solved

nickassink commented 3 weeks ago

Does the workaround that @nickassink mentioned work for you?

In short yes, now it will run test cases one by one, so you don't have a massive command to run them all.

I will just wait until you fix it for @ebouteillon and test again I think everything will work then. if it does not I will create a separate issue (don't want to highjack this one with a different issue).

ebouteillon commented 3 weeks ago

Latest CMake (3.29) has a new command-line option to list the tests to run in a file: https://cmake.org/cmake/help/latest/manual/ctest.1.html#cmdoption-ctest-tests-from-file

If the extension uses this option it should solve the issue for "CMake >= 3.29" users. Not a problem for me, as I can upgrade the CMake version. If you implement it, I can give a feedback.

gcampbell-msft commented 3 weeks ago

@ebouteillon Thanks for testing, could you try this vsix as well?

cmake-tools.zip

Some context into my attempted fixes is that in order to properly support parallel invocation when being invoked by the test explorer, we have to make sure we support if there is a subset selected in the test explorer. First I tried to be nifty in how I did that, but in this fix I'm being more straightforward. Please let me know if this vsix helps your scenario.

ebouteillon commented 3 weeks ago

Hi @gcampbell-msft , new .vsix works for my use case. Command line generated no longer lists all tests cases.

v-frankwang commented 3 weeks ago

@ebouteillon Thank you very much for your reply, waiting for this Pull request: https://github.com/microsoft/vscode-cmake-tools/pull/3814 to complete and the problem will be fixed!

triou-harmonicinc commented 3 weeks ago

@gcampbell-msft Unfortunately I ran into the same issue using a subset of the test through the TestExplorer and your fix won't tackle that (gtest big parametric test names does not help). Is there any way to check the size of the regex and split into several commands to avoid the issue ? Or perhaps by looking up the test numbers, the ctest command could be run with -I instead of -R Of course the CMake 3.29 --tests-from-file new command line would be the cleanest solution but it requires a CMake upgrade for the user.

ebouteillon commented 3 weeks ago

Hi @triou-harmonicinc , I am using NO_PRETTY_TYPES and NO_PRETTY_VALUES with gtest_discover_tests to have shorter gtest test names. Maybe you can give a try?

gcampbell-msft commented 3 weeks ago

@triou-harmonicinc Thanks for the feedback!

The previous experience wasn't actually making use of the cmake.ctest.allowParallelJobs setting. Any subset selection of tests was invoked individually. This change was made in order to allow for a subset selection of tests to make use of the cmake.ctest.allowParallelJobs setting.

I agree that our current implementation still definitely has limitations, but that will be a future enhancement. For now, if you want to be able to run a subset, you should disable the cmake.ctest.allowParallelJobs setting, and it will behave the same way as before (specifically for a subset ran from the Test Explorer). @triou-harmonicinc Could you help us out by creating a separate enhancement request?

The overall issue where invoking all tests from outside of the Test Explorer, will be patched into a patch 1.18 release.

gcampbell-msft commented 3 weeks ago

@ebouteillon The fix will be in the pre-release channel tomorrow! Also, we plan to do a patch official release next week. THanks!

triou-harmonicinc commented 3 weeks ago

@gcampbell-msft Sounds good. Should I create a new issue to improve the behavior of cmake.ctest.allowParallelJobs then ? (I could not find an existing one)

triou-harmonicinc commented 3 weeks ago

@gcampbell-msft I did test your latest changes and it breaks running a subset of tests with parallel jobs enabled. Instead of running just the subset with the huge regex it runs all tests. Was that intended as well ?

gcampbell-msft commented 3 weeks ago

@gcampbell-msft I did test your latest changes and it breaks running a subset of tests with parallel jobs enabled. Instead of running just the subset with the huge regex it runs all tests. Was that intended as well ?

Could you provide some clarity on the repro you tried?

I don't see the break that you speak of when I tested on my machine:

https://github.com/microsoft/vscode-cmake-tools/assets/86264750/96a2dd99-0008-4687-bd2c-85b1f65965a4

gcampbell-msft commented 3 weeks ago

If you're referring to the button here, I don't think this is the intent of the Test Explorer, when debugging, it doesn't include information about the subset:

https://github.com/microsoft/vscode-cmake-tools/assets/86264750/554b29e5-a869-4db1-afdb-666d08cbd770

In which case, this wouldn't be a regression. Also, I tested in 1.17.17 and this behavior was there as well.

gcampbell-msft commented 3 weeks ago

@gcampbell-msft Sounds good. Should I create a new issue to improve the behavior of cmake.ctest.allowParallelJobs then ? (I could not find an existing one)

Yes, please feel free to create a new issue for any request you have for improved behavior. Thanks!

triou-harmonicinc commented 3 weeks ago

My use case is :

enter a filter
click on the button next to the project name

I double checked and if I revert your changes the old behavior is back.

gcampbell-msft commented 3 weeks ago

@triou-harmonicinc This is what I see:

https://github.com/microsoft/vscode-cmake-tools/assets/86264750/dae9b8f4-8e4a-40e3-92c7-3fa686fee5c1

Is there something else I'm missing?

nickassink commented 3 weeks ago

I will just wait until you fix it for @ebouteillon and test again I think everything will work then. if it does not I will create a separate issue (don't want to highjack this one with a different issue).

@gcampbell-msft I just tested it with te pre release and this also fixed my issue. thank you.

triou-harmonicinc commented 2 weeks ago

@gcampbell-msft Sorry for the delayed answer, I am trying to debug the issue. It seems to depend on the filter I am using which I find very peculiar.

triou-harmonicinc commented 2 weeks ago

The issue comes from the TestRunRequest.include field which is ont correctly filled with a specific filter. So the bug might come from the test explorer extension API itself. It might also be linked to my configuration. Anyway, thanks for your support I will try to look into it more deeply and report to the right place if I can figure out what's wrong.

microsoft / vscode-cmake-tools