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

Regression on running CTest with v1.18.41 update #3804

Closed
ebouteillon opened this issue May 31, 2024 · 24 comments · Fixed by #3814
Closed

Regression on running CTest with v1.18.41 update #3804

ebouteillon opened this issue May 31, 2024 · 24 comments · Fixed by #3814
Labels
bug a bug in the product Feature: ctest regression used to work and no longer does. Regressions are typically high priority
Milestone

Comments

@ebouteillon
Copy link

ebouteillon commented May 31, 2024

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

@github-project-automation github-project-automation bot moved this to Triage Needed in CMake Tools May 31, 2024
@ebouteillon ebouteillon changed the title Regression on running CTest Regression on running CTest with v1.18.41 update May 31, 2024
@nickassink
Copy link

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.

image

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

@ebouteillon
Copy link
Author

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

@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
Copy link
Collaborator

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

@gcampbell-msft gcampbell-msft added regression used to work and no longer does. Regressions are typically high priority bug a bug in the product Feature: ctest and removed triage labels Jun 3, 2024
@gcampbell-msft gcampbell-msft moved this from Triage Needed to In Progress in CMake Tools Jun 3, 2024
@gcampbell-msft gcampbell-msft added this to the On Deck milestone Jun 3, 2024
@ebouteillon
Copy link
Author

ebouteillon commented Jun 4, 2024

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

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

ebouteillon commented Jun 4, 2024

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

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

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

@v-frankwang
Copy link
Collaborator

@ebouteillon Thank you very much for your reply, waiting for this Pull request: #3814 to complete and the problem will be fixed!

@triou-harmonicinc
Copy link

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

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?

image

@gcampbell-msft
Copy link
Collaborator

gcampbell-msft commented Jun 5, 2024

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

@github-project-automation github-project-automation bot moved this from In Progress to Completed in CMake Tools Jun 5, 2024
@gcampbell-msft
Copy link
Collaborator

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

triou-harmonicinc commented Jun 6, 2024

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

@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
Copy link
Collaborator

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

CTest_Subset.mp4

@gcampbell-msft
Copy link
Collaborator

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:

CTest_Subset_Top_Button.mp4

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

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

triou-harmonicinc commented Jun 6, 2024

My use case is :

  1. enter a filter
  2. click on the button next to the project name
    regression

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

@gcampbell-msft
Copy link
Collaborator

@triou-harmonicinc This is what I see:

CTestSelect.mp4

Is there something else I'm missing?

@gcampbell-msft gcampbell-msft modified the milestones: On Deck, 1.18 Jun 6, 2024
@nickassink
Copy link

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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a bug in the product Feature: ctest regression used to work and no longer does. Regressions are typically high priority
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

5 participants