numaru / vscode-ceedling-test-adapter

Ceedling Test Adapter for the VS Code Test Explorer
MIT License
37 stars 14 forks source link

Test explorer not working with parameterized tests #67

Closed LeMimit closed 2 years ago

LeMimit commented 3 years ago

Hello !

I have the following test:

/**
 * @brief Check that we handle incorrect values
 */
TEST_CASE( "0x123", 10)
TEST_CASE( "abc", 10)
void test_convertStrToSignedNumber_invalid_input(char *input, uint8_t base)
{
  long value;
  enum app_ecode_e rc;

  rc = convertStrToSignedNumber(input, &value, base);
  TEST_ASSERT_EQUAL( rc, APP_ECODE_BAD_PARAMETER );

}

When I am trying to run it, the status is not updated in the test explorer interface. The test is a success in the report.

image

I do not have any problems with tests which are not parameterized.

image

Seems like parameterized tests is not supported by the extensions ? May be I need to add some options ?

Thanks a lot !

numaru commented 3 years ago

The parameterized tests are not yet available in the extension.

The way the extention works currently:

I don't think there is a way to get a list of all the tests from the ceedling command line without running them. This is why the extension use a regex.

In the report.xml, the parameterized tests are named with a suffix. For example, test/TestFoo.c::test_convertStrToSignedNumber_invalid_input("0x123", 10).

To support the parameterized test in the extension, I think we should add an entry in the tree view for each new params. I'm not sure if it should be done after the test run using the report.xml (need to check if it can be done as a test adapter) or with the regex (what happen if someone use another macro than TEST_CASE as it is in this workaround? https://github.com/ThrowTheSwitch/Ceedling/issues/241, we should make this configurable).

LeMimit commented 3 years ago

Ok, thanks for the answer.

Seems like it should be more intuitive to have the parameterized test appears after the discovery of the tests, so it should be done with the regex.

Do you plan to add this feature in the near futur ?

If no, may be I can help you on this ? How do you mange this ? Can you give me some more information on how modify the architecture to add this feature ?

Thanks !

fj-sanchez commented 3 years ago

It would be great to get this feature. I think that not being able to detect all the tests cases before execution is fine, you can always retrieve the full list after execution from the XML, as these are easy to match, and add these as a tree below the item.

I think this is possible according to this https://github.com/hbenl/vscode-test-explorer/issues/165

numaru commented 3 years ago

I'm looking at this

LeMimit commented 3 years ago

Thanks for adding this feature! πŸ‘πŸ» I will test it tomorrow πŸ˜„

fj-sanchez commented 3 years ago

Thanks @numaru for adding it, I'll also try it tomorrow and provide feedback.

clabough commented 3 years ago

Will this also make it so that this shows pass/fail for each test instead of a gray circle with square inside?

  describe.each([['0'], ['-1']])('should invalidate integer length values less than 1', badValue => {
    it(` "${badValue}"`, () => {
      expect(component.validNumber(badValue)).toBe(false);
    });
  });
numaru commented 3 years ago

@clabough Sorry, I’m afraid I don’t follow you. I don't understand what you would expect and when. And why your example it written in javascript? This repo handle the ceedling adaption to the VSCode test explorer extension πŸ˜‰

clabough commented 3 years ago

@numaru Sorry, I ended up here somehow looking into an issue with Jest Test Explorer/Test Explorer UI. My mistake.

olowo726 commented 2 years ago

This seems to be implemented but since the issue isn't closed I'll write a feature request here.

We have to use the workaround described at https://github.com/ThrowTheSwitch/Ceedling/issues/195. There is a solution to avoid this workaround (https://github.com/ThrowTheSwitch/Ceedling/pull/497) but in involves compiler-specific flags for modifying macro processing behaviour. This workaround breaks our existing test cases.

Hence, could I please have an extension setting enabling me to tell the extension to use TEST_VALUE (or arbitrary string) instead of TEST_CASE? The same applies to TEST_RANGE.

numaru commented 2 years ago

This seems to be implemented but since the issue isn't closed I'll write a feature request here.

We have to use the workaround described at ThrowTheSwitch/Ceedling#195. There is a solution to avoid this workaround (ThrowTheSwitch/Ceedling#497) but in involves compiler-specific flags for modifying macro processing behaviour. This workaround breaks our existing test cases.

Hence, could I please have an extension setting enabling me to tell the extension to use TEST_VALUE (or arbitrary string) instead of TEST_CASE? The same applies to TEST_RANGE.

new issue opened here https://github.com/numaru/vscode-ceedling-test-adapter/issues/91