Add JUnit XML support #337

trevershick · 2021-09-19T01:50:29Z

Added an srunner_set_xml_format function and also an CK_XML_FORMAT_NAME environment variable to select JUnit formatting
Test timings are not supported at this time
Add tests to cover new functionality
Update documentation

The implementation was inspired by Glenn Washburn's original patches.

references -
Original Patches - https://sourceforge.net/p/check/mailman/message/2963561/
Issue #334

* Added an srunner_set_xml_format function and also an CK_XML_FORMAT_NAME environment variable to select JUnit formatting * Test timings are not supported at this time * Add tests to cover new functionality * Update documentation The implementation was inspired by Glenn Washburn's original patches. references - Original Patches - https://sourceforge.net/p/check/mailman/message/2963561/

trevershick · 2021-09-25T02:13:12Z

My CircleCI job for a pet project is using the patched version of check based on these changes. You can see the results files on any given build in https://app.circleci.com/pipelines/github/trevershick/wpp under the tests tab ( a recent build: https://app.circleci.com/pipelines/github/trevershick/wpp/48/workflows/8d0d5d1a-8fc2-4eac-95aa-bae3f84435a5/jobs/48/tests ). I can't guarantee that build will be there when you look though.

trevershick · 2021-09-25T02:24:46Z

@mikkoi Any tips for getting a maintainer to run the workflows? I'd like to contribute more.

brarcher · 2021-10-22T06:54:08Z

Hm. I did not realize that the tests would not run automatically for first-time collaborators. That must be a recent change:
https://github.blog/changelog/2021-04-22-github-actions-maintainers-must-approve-first-time-contributor-workflow-runs/

On another PR I saw this week there was a "run workflow" button I could click to start the workflows. However, on this PR there is no such button.

Does the branch need to be up-to-date with the master branch to have the option to run the workflow? The docs do not mention this as a requirement:

https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

trevershick · 2021-10-22T12:12:10Z

Thanks for looking at this @brarcher . I've brought the PR up to date with master, perhaps this will allow the workflows to be run.

brarcher · 2021-10-23T17:07:30Z

Ah, that was is, thanks. The test workflows did run, though a number of them hit test failures. The tests are passing on the master branch, so I don't believe the test failures are pre-existing.

brarcher · 2021-10-23T17:08:39Z

doc/check.texi

@@ -2048,6 +2052,59 @@ If both plain text and XML log files are specified, by any of above methods,
 then check will log to both files. In other words logging in plain text and XML
 format simultaneously is supported.

+JUnit Support is also available.  It is enabled  by a call to 


Is it "JUnit support" or "JUnit XML support" ?

Addressed in 2e7563c

brarcher · 2021-10-23T17:16:49Z

doc/check.texi

@@ -2048,6 +2052,59 @@ If both plain text and XML log files are specified, by any of above methods,
 then check will log to both files. In other words logging in plain text and XML
 format simultaneously is supported.

+JUnit Support is also available.  It is enabled  by a call to 
+@code{srunner_set_xml_format(CK_XML_FORMAT_JUNIT)} before the tests are run. 
+It can also be enabled by environment variable as well.  It is enabled by setting the


Could combine two sentences this way:

It can also be enabled by setting the environment varaible @code{CK_XML_FORMAT_NAME} to @code{junit}.

Also, if a new environment variable is being added, add a reference here:
https://github.com/libcheck/check/blob/master/doc/check.texi#L2286

Addressed in 2e7563c

brarcher · 2021-10-23T17:17:45Z

doc/check.texi

+It can also be enabled by environment variable as well.  It is enabled by setting the
+@code{CK_XML_FORMAT_NAME} environment variable to @code{junit}.
+
+Here is an example of the JUnit xml format:


Addressed in 2e7563c

brarcher · 2021-10-23T17:21:42Z

src/check.h.in

+ * enum describing the specific XML format used for XML logging
+ */
+enum xml_format {
+    CK_XML_FORMAT_UNSPECIFIED,


should the unspecified state be the default state? What is the motivation for having an unspecified state? And, if it were unspecified what XML stlye ends up being used?

CK_XML_FORMAT_UNSPECIFIED is the default value when srunner_create is called. This is in fact what I was missing and why my tests were failing.

srunner_xml_format is used to determine what format to use.
srunner_xml_format will never yield CK_XML_FORMAT_UNSPECIFIED, it will yield only CK_XML_FORMAT_DEFAULT or CK_XML_FORMAT_JUNIT.

The motivation for the unspecified state is that I needed an initial state which was unspecified since the environment variable to set the format only overrides the selected format if it's unspecified. If it were set to 'default', then I couldn't tell if someone had set it explicitly or not.

brarcher · 2021-10-23T17:24:29Z

src/check.h.in

+ * This value can be explicitly set via `srunner_set_xml_format` or can
+ * be set via the CK_XML_FORMAT_NAME environment variable.  
+ *
+ * @return CK_XML_FORMAT_DEFAULT unless the format is explicitly set via


If another XML style were introduced in the future, the docs here would need to be updated to remove the JUNIT reference (as there would be other possibilities).

Perhaps mention how the environment variable and value set in the code interact instead, so that is more clear. For example, mention that if the environment variable is set then its value is used, otherwise whatever is set in srunner_set_xml_format is used, and the default is CK_XML_FORMAT_DEFAULT.

addressed in 2e7563c

brarcher · 2021-10-23T17:28:37Z

src/check_log.c

+
+    // junit is the only value of CK_XML_FORMAT_NAME that will
+    // return something other than CK_XML_FORMAT_DEFAULT
+    const char *format_name = getenv("CK_XML_FORMAT_NAME");


I think in other places in the code the environment variable takes precedence. Example:

https://github.com/libcheck/check/blob/master/src/check.c#L317

Should that be the case here as well?

Since it's logging, I made the precedence the same as srunner_xml_fname. This seems sane to me even if it's different than other items. These two should at least agree probably.

brarcher · 2021-10-24T16:28:16Z

src/check_impl.h

@@ -121,6 +122,7 @@ struct SRunner
    List *resultlst;            /* List of unit test results */
    const char *log_fname;      /* name of log file */
    const char *xml_fname;      /* name of xml output file */
+    enum xml_format xml_format;


there are comments on the other fields, maybe add one for this field too?

Addressed in 2e7563c

brarcher · 2021-10-24T16:30:55Z

src/check_print.c

+                 enum print_output print_mode CK_ATTRIBUTE_UNUSED)
+{
+    char status[10];
+    char type[10];


I don't see type used in the function

type was removed in f6d5589

trevershick · 2021-10-29T23:57:03Z

Got a linux box up and going - got a clean test run:

$ CTEST_OUTPUT_ON_FAILURE=1 ninja -C build test
ninja: Entering directory `build'
[0/1] Running tests...
Test project /home/trevershick/workspace/check/build
    Start 1: check_check
1/9 Test #1: check_check ......................   Passed  178.07 sec
    Start 2: check_check_export
2/9 Test #2: check_check_export ...............   Passed  176.53 sec
    Start 3: test_output.sh
3/9 Test #3: test_output.sh ...................   Passed    0.11 sec
    Start 4: test_log_output.sh
4/9 Test #4: test_log_output.sh ...............   Passed    0.03 sec
    Start 5: test_xml_output.sh
5/9 Test #5: test_xml_output.sh ...............   Passed    0.09 sec
    Start 6: test_tap_output.sh
6/9 Test #6: test_tap_output.sh ...............   Passed    0.03 sec
    Start 7: test_check_nofork.sh
7/9 Test #7: test_check_nofork.sh .............   Passed    0.01 sec
    Start 8: test_check_nofork_teardown.sh
8/9 Test #8: test_check_nofork_teardown.sh ....   Passed    0.01 sec
    Start 9: test_set_max_msg_size.sh
9/9 Test #9: test_set_max_msg_size.sh .........   Passed    0.02 sec

100% tests passed, 0 tests failed out of 9

Total Test time (real) = 354.89 sec

* JUnit Support -> JUnit XML Support * Combine two sentences in check.texi * xml->XML * Add a line in the environment appendix * Update the xml_format source code header comment * Add a comment to xml_format

trevershick · 2022-01-22T14:49:08Z

I just noticed this is still open. I believe I addressed most comments. There are a couple where I attempted to explain my reasoning.

Trever Shick added 2 commits September 18, 2021 20:42

add missing free, remove unused variable

f6d5589

trevershick mentioned this pull request Sep 19, 2021

Glenn Washburn's JUnit patch was never merged back then. What happened? #334

Open

Merge remote-tracking branch 'origin/master' into tshick-334

20efb01

brarcher reviewed Oct 23, 2021

View reviewed changes

brarcher requested changes Oct 24, 2021

View reviewed changes

trevershick requested a review from brarcher October 30, 2021 00:03

Trever Shick added 2 commits October 29, 2021 19:36

Initialize xml_format in srunner_create

ee51d1c

Update documentation

2e7563c

* JUnit Support -> JUnit XML Support * Combine two sentences in check.texi * xml->XML * Add a line in the environment appendix * Update the xml_format source code header comment * Add a comment to xml_format

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JUnit XML support #337

Add JUnit XML support #337

trevershick commented Sep 19, 2021

trevershick commented Sep 25, 2021 •

edited

Loading

trevershick commented Sep 25, 2021 •

edited

Loading

brarcher commented Oct 22, 2021

trevershick commented Oct 22, 2021

brarcher commented Oct 23, 2021

brarcher Oct 23, 2021

trevershick Oct 29, 2021

brarcher Oct 23, 2021

trevershick Oct 29, 2021

brarcher Oct 23, 2021

trevershick Oct 29, 2021

brarcher Oct 23, 2021

trevershick Oct 29, 2021

brarcher Oct 23, 2021

trevershick Oct 29, 2021

brarcher Oct 23, 2021

trevershick Oct 29, 2021

brarcher Oct 24, 2021

trevershick Oct 29, 2021

brarcher Oct 24, 2021

trevershick Oct 30, 2021

trevershick commented Oct 29, 2021

trevershick commented Jan 22, 2022

Add JUnit XML support #337

Are you sure you want to change the base?

Add JUnit XML support #337

Conversation

trevershick commented Sep 19, 2021

trevershick commented Sep 25, 2021 • edited Loading

trevershick commented Sep 25, 2021 • edited Loading

brarcher commented Oct 22, 2021

trevershick commented Oct 22, 2021

brarcher commented Oct 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevershick commented Oct 29, 2021

trevershick commented Jan 22, 2022

trevershick commented Sep 25, 2021 •

edited

Loading

trevershick commented Sep 25, 2021 •

edited

Loading