search

orjangj / neotest-ctest

A neotest adapter for C/C++ using CTest as a test runner. Supports GoogleTest, Catch2 and doctest test frameworks.
MIT License
8 stars 4 forks source link

parsing error using INSTANTIATE_TEST_SUITE_P #38

Open pcorbineau opened 21 hours ago

pcorbineau commented 21 hours ago

There is probably an error during test name parsing and interpretation using INSTANTIATE_TEST_SUITE_P.

Here is my cpp test code : 

INSTANTIATE_TEST_SUITE_P(
    StandardFilesWrite,
    MultipleFileWriteTests,
    testing::Values(
        FileWriter::BUFFER_SIZE,
        FileWriter::BUFFER_SIZE * 2,
        FileWriter::BUFFER_SIZE * 4,
        FileWriter::BUFFER_SIZE * 10,
        0ul,
        1ul,
        500ul,
        1000ul,
        5000ul,
        10000ul,
        100000ul,
        500000ul,
        1000000ul,
        10000000ul));

My test are well recognized for neotest summary but are not found for ctest instantiation. Here is my investigation, It seams that ctest --show-only are interpreting code from GTEST macro :

$ ctest --test-dir build\src --show-only
...
  Test #248: StandardFilesWrite/MultipleFileWriteTests.WriteFile/65536
  Test #249: StandardFilesWrite/MultipleFileWriteTests.WriteFile/131072
  Test #250: StandardFilesWrite/MultipleFileWriteTests.WriteFile/262144
  Test #251: StandardFilesWrite/MultipleFileWriteTests.WriteFile/655360
  Test #252: StandardFilesWrite/MultipleFileWriteTests.WriteFile/0
  Test #253: StandardFilesWrite/MultipleFileWriteTests.WriteFile/1
  Test #254: StandardFilesWrite/MultipleFileWriteTests.WriteFile/500
  Test #255: StandardFilesWrite/MultipleFileWriteTests.WriteFile/1000
  Test #256: StandardFilesWrite/MultipleFileWriteTests.WriteFile/5000
  Test #257: StandardFilesWrite/MultipleFileWriteTests.WriteFile/10000
  Test #258: StandardFilesWrite/MultipleFileWriteTests.WriteFile/100000
  Test #259: StandardFilesWrite/MultipleFileWriteTests.WriteFile/500000
  Test #260: StandardFilesWrite/MultipleFileWriteTests.WriteFile/1000000
  Test #261: StandardFilesWrite/MultipleFileWriteTests.WriteFile/10000000
  ...

My debugging investigation with some prints give me this : in lua/neotest-ctest/init.lua:58 

table.insert(runnable_tests, testcases[node.name])

node.namedoesn't correspond with testcases since one is interpreted and the other is not. Here is the printed list of non interpreted tests from node.name variable

StandardFilesWrite/MultipleFileWriteTests.WriteFile/FileWriter::BUFFER_SIZE
StandardFilesWrite/MultipleFileWriteTests.WriteFile/FileWriter::BUFFER_SIZE * 2
StandardFilesWrite/MultipleFileWriteTests.WriteFile/FileWriter::BUFFER_SIZE * 4
StandardFilesWrite/MultipleFileWriteTests.WriteFile/FileWriter::BUFFER_SIZE * 10
StandardFilesWrite/MultipleFileWriteTests.WriteFile/0ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/1ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/500ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/1000ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/5000ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/10000ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/100000ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/500000ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/1000000ul
StandardFilesWrite/MultipleFileWriteTests.WriteFile/10000000ul

if anybody has idea how to deal with this issue, I'm open to fix issue.

orjangj commented 17 hours ago

Pretty sure we've hit a limitation with simple text-based parsing that neotest-ctest currently relies on (through treesitter). I didn't really consider literal suffixes, named constants, or even preprocessor macros for that matter when I started working on supporting parameterized gtest tests. So the implementation is pretty naive in that respect.

I know that gtest_discover_tests will run the test executable (as part of execute_process) in order to enumerate all the test cases for ctest. So I assume the test executable reports the test cases with their values replaced by their numeric/actual values, which should explain the output you see from running ctest --test-dir build\src --show-only.

Given your example, I think neotest-ctest should be able to parse StandardFilesWrite/MultipleFileWriteTests.WriteFile part of the parameterized tests reliably at least, but for the test case values it might be better if neotest-ctest just queried ctest and perfomed a match (with StandardFilesWrite/MultipleFileWriteTests.WriteFile as a match criteria) to parse the actual test case names reported by the gtest test executable to ctest.

As far as I can tell, there's two disadvantages to this method though: (1) Querying ctest as part of neotest's discover_positions may have a negative performance impact on test discovery, and (2) the parameterized test cases in the neotest summary would show up as:

StandardFilesWrite/MultipleFileWriteTests.WriteFile/65536
StandardFilesWrite/MultipleFileWriteTests.WriteFile/131072
StandardFilesWrite/MultipleFileWriteTests.WriteFile/262144
...
StandardFilesWrite/MultipleFileWriteTests.WriteFile/1000000
StandardFilesWrite/MultipleFileWriteTests.WriteFile/10000000

instead of the more verbose naming in your code.

Nevertheless, you should at least be able to run the individual test cases from the neotest summary. Which is better than not being able to run parameterized tests at all :sweat_smile:

While thinking on it, this approach might even work for the other parameter generators currently not supported by neotest-ctest as well. Would be nice if it did.

I don't have much time at the moment to have a look at this, but if you want to have a go at it then that would be much appreciated. I would start looking at neotest-ctest/lua/framework/gtest.lua and in particular the function gtest.build_parameterized. you would need to require("neotest-ctest.ctest") in order to query the test cases.