Adding output to JUnit.xml with JUnit schema files. #27

QGhd · 2023-07-31T11:24:16Z

I have attached two changed source files (replacing src/testdrive.f90 and test/main.f90) as well as the resulting JUnit.xml file that is written along with the screen output. This xml is doing the job for us in an Azure DevOps pipeline based on the data that is also written on the screen.
The main.f90 uses 4 additional external subroutines from testdrive.f90. Two to open and close the Junit.xml and two to open and close the xml tag. The rest of the xml output is covered internally in testdrive.f90.
However, I assume that you would prefer to avoid these additional functions, but I choose a simple approach with minimal code changes, instead of a major rework of the module structure.

In addition, the JUnit.xml file references to an xml schema (JUnit.xsd from https://github.com/windyroad/JUnit-Schema). In theory the JUnit.xml should be conform to this schema, but it is not for several reasons:

There are missing (required) xml tag attributes, like number of all/failed/succeeded/skipped tests per testsuite that are as summarized data not available at the time, when the according opening xml tag is written.
There are missing xml tag attributes, like execution time, that are not stored in the code (as far as I could see).
There is no corresponding xml tag in the schema file for the additional stdout/stderr output per testcase. Therefore, I added according xml tags () per testcase that are - following the xsd - only allowed once per testsuite.
Fortunately, Azure DevOps does not care about the discrepancy, so we are fine with this setup (but it is not a generic solution).
I have also attached another xml schema file (jenkins-junit.xsd from https://github.com/junit-team/junit5 resp. https://junit.org/junit5/). To be honest I am not sure, what should be regarded as the standard for a JUnit.xml. In fact, I do not care much, since Azure DevOps test pipeline works fine. Others, however, may have a more specific opinion on the subject.

Let me know, if you want to discuss my changes or whether I can support you.
Thank you for considering my changes.

jvdp1 · 2023-08-01T19:59:50Z

Interesting. the JUnit.xml file could be acutally also used in the CI/CD of Gitlab. The current generated xml file is already recognized by Gitlab (still a few issues with the name of the suite not recognized by Gitlab). Execution time would be useful too.

src/testdrive.F90

jvdp1 · 2023-08-01T20:13:46Z

src/testdrive.F90

+
+    stdout = ''
+    if (allocated(error)) then
+      stdout = trim(error%message)
+    endif
+
+    s = index(message, '[')
+    e = index(message, ']')
+    res = message(s+1:e-1)
+    select case (res)
+      case ('FAILED', 'UNEXPECTED PASS')
+        call junitxml_write_testcase_opening_tag(test%name)
+        call junitxml_write_testcase_failure(res)
+        call junitxml_write_testcase_closing_tag(stdout)
+      case ('EXPECTED FAIL', 'PASSED')
+        if (len_trim(stdout) > 0) then  
+          call junitxml_write_testcase_opening_tag(test%name)
+          call junitxml_write_testcase_closing_tag(stdout)
+        else
+          call junitxml_write_testcase_single_tag(test%name)
+        endif
+      case ('SKIPPED')
+        res = res
+      case default
+        write(unit, '(a)') "Error: Unknown test result '" // res // "' in test '" // test%name // "'!"
+        stop 2
+    end select
+


Note that this section is in an OpenMP parallel section. Therefore, I/O may no be sequential (is it important?). Furthermore, I think it should be in a critical section

I am not sure, what is the best solution. I am not experienced with OpenMP.
Wrapping the calls in
!$omp critical(testdrive_testsuite) call junitxml_...() !$omp end critical(testdrive_testsuite) seems too be quite coarse.
I could add the !$omp critical ... around any write statement inside the calls.
But does this resolve the OpenMP parallel issue?

Please, let me know your thoughts.

.gitignore

awvwgk · 2023-08-02T07:21:52Z

src/JUnit.xml

Should this file be checked in?

I have added for you (resp. the reviewers). It documents the expected output. Could be used for your documentation. Could also be used, if you would like to implement a test. But it is not relevant for the build and can be deleted, if you want.

@awvwgk: Do you have preference, whether to delete this file or not?
It is your project, so I will follow your choice.

Please, let me know your thoughts.

awvwgk · 2023-08-02T07:22:15Z

src/JUnit.xsd

Same for this file, should this be checked in?

If the JUnit.xml references the JUnit.xsd in line 2. It should be included and maybe install in the output directory of the JUnit.xml. It might be useful to add it to the documentation.
If the schema reference is removed from JUnit.xml, it might be sufficient to reference the file in the documentation.

@awvwgk: Do you have preference, whether to delete this file or not?
It is your project, so I will follow your choice.

Please, let me know your thoughts.

awvwgk · 2023-08-02T07:22:37Z

src/jenkins-junit.xsd

This also seems to be checked in by accident

Also for documentation resp. for you. This schema file might be an alternative for someone, how prefers this schema file for Jenkins. I cannot test it. If you prefer, I can delete any of these files from the PR.

@awvwgk: Do you have preference, whether to delete this file or not?
It is your project, so I will follow your choice.

Please, let me know your thoughts.

awvwgk

Thanks, this looks pretty straight forward to add and I would be happy to accept this in test-drive. One general comment, the generation of the JUnit.xml is incremental, which means if the test run is aborted the partly generated JUnit.xml file is always invalid. Could we have a process which guarantees that the JUnit.xml file can only be written completely?

QGhd · 2023-08-02T10:11:37Z

Thanks, this looks pretty straight forward to add and I would be happy to accept this in test-drive. One general comment, the generation of the JUnit.xml is incremental, which means if the test run is aborted the partly generated JUnit.xml file is always invalid. Could we have a process which guarantees that the JUnit.xml file can only be written completely?

Good point. Originally, I tried to use a FINAL statement for the module to make sure that the final closing tag is written and the xml file is closed, even in the error case. However, I failed. My final subroutine was never executed.
In any case the nested structure of the xml file requires either to track the current depth, when ABORT, ERROR, ... occurs or to use one FINAL statement per interface of the module.
What do you think?

FYI: https://fortran-lang.discourse.group/t/when-a-final-subroutine-is-called/3246

jvdp1 · 2023-08-03T16:54:42Z

src/testdrive.F90

+        call junitxml_write_testcase_opening_tag(test%name)
+        call junitxml_write_testcase_failure(res)
+        call junitxml_write_testcase_closing_tag(stdout)
+      case ('EXPECTED FAIL', 'PASSED')
+        if (len_trim(stdout) > 0) then  
+          call junitxml_write_testcase_opening_tag(test%name)
+          call junitxml_write_testcase_closing_tag(stdout)
+        else
+          call junitxml_write_testcase_single_tag(test%name)
+        endif
+      case ('SKIPPED')
+        res = res
+      case default
+        write(unit, '(a)') "Error: Unknown test result '" // res // "' in test '" // test%name // "'!"
+        error stop 2
+    end select


Maybe a suggestion to avoid the issue with I/O in the parallel loop:

Transform the subroutines junit_write_testcase_* in functions returning charachter variables, concatanate them in one single character variable, and write it in a critical section. This will also avoid the incremental issue raised by @awvwgk

this could be additionally extended to the whole JUnit.xml file, that is the content of the JUnit.xml file will be first saved in a character variable, and be written to the file only if completed.

Sorry, for not responding earlier. I was on holiday the last weeks.
I looked into the proposal to return a string instead of write it to the file. But I am not sure, whether it is a pragmatic way forward. I see two options:

In FORTRAN 2003+ one can use dynamic allocatable strings on every level of the code (see https://stackoverflow.com/questions/14886787/fortran-character-input-at-undefined-length, https://stackoverflow.com/questions/31084087/using-a-deferred-length-character-string-to-read-user-input).

In older FORTRAN (< 2003) the string size needs to be managed on gloabl scope to grow with the amount of test output, since the total size of the output is unknown in advance.

Option 1 is efficient and light-weigth, but it would possibly exclude too many applications using FORTRAN < 2003.
Option 2 is laborious with all the book keeping.

Do you favor one of the above options or do see another way?
Please, let me know your thoughts.

My suggestion is to use the Option 1 (i.e. character(len=:), allocatable variable). Applications written in older Fortran could still use this library, provided that the used compiler support Fortran 2003+.

jvdp1 · 2023-08-03T16:55:18Z

src/testdrive.F90

+    s = index(message, '[')
+    e = index(message, ']')
+    res = message(s+1:e-1)
+    select case (res)


Does this mean that this select case also happen, even if the user does not want JUnit.xml file?

At the moment the JUnit.xml is written always.

Up to @awvwgk to decide the final behaviour, but my preference would be htat JUnit.xml is written only on request of the user.

My suggestion is that calling junitxml_open_file() switches a global variable from .false. to .true.. If .true. the output file is written. This requires that any of the junitxml_write_*() routines needs to respect this flag.
Let me know, if you propose a different way.
If not, I will implement as suggested.
Please, let me know your thoughts.

That is an option.
Another possible option is to store the content in an optional character variable, and to write this variable at the end of all tests.
The optional variable would allow backward compatibility.

E.g.,

character(len=:), allocatable :: xml_ write(error_unit, fmt) "Suite:", testsuites(is)%name call run_selected(testsuites(is)%collect, test_name, error_unit, stat, xml = xml_) call junitxml_write_testsuite(xml_)

src/testdrive.F90

jvdp1 · 2023-08-08T06:26:39Z

src/testdrive.F90

+        call junitxml_write_testcase_opening_tag(test%name)
+        call junitxml_write_testcase_failure(res)
+        call junitxml_write_testcase_closing_tag(stdout)
+      case ('EXPECTED FAIL', 'PASSED')
+        if (len_trim(stdout) > 0) then  
+          call junitxml_write_testcase_opening_tag(test%name)
+          call junitxml_write_testcase_closing_tag(stdout)
+        else
+          call junitxml_write_testcase_single_tag(test%name)


All these subroutines could be combined into a single one:

call junitxml_write_testcase(test%name, res, stdout)

with res and stdout being optional.

it could even be a function to be used as:

!$omp critical select case(res) case(...) write(unit_junitxm, '(a)') junitxml_write_testcase(test%name, res, stdout) case(...) end select !$end omp critical

Such an approach (with a function returning a character string, instead of writing it directly to a file) could solve the issue raised by @awvwgk

I am not sure about the dynamic size of the string variable. Please, refer to my response here: #27 (comment).
Please, let me know your thoughts.

QGhd · 2023-09-04T20:09:38Z

@awvwgk: With respect to your change request:

Thanks, this looks pretty straight forward to add and I would be happy to accept this in test-drive. One general comment, the generation of the JUnit.xml is incremental, which means if the test run is aborted the partly generated JUnit.xml file is always invalid. Could we have a process which guarantees that the JUnit.xml file can only be written completely?

A delayed output of the file at the end of application is fmho not a reasonable option (see #27 (comment)). I can try to delete the file, if it could not be written completely in case of an error.
Please, let me know your thoughts.

Variable declaration changed according reviewer's suggestion. Co-authored-by: Jeremie Vandenplas <[email protected]>

jvdp1 · 2023-09-05T07:09:48Z

@awvwgk: With respect to your change request:

Thanks, this looks pretty straight forward to add and I would be happy to accept this in test-drive. One general comment, the generation of the JUnit.xml is incremental, which means if the test run is aborted the partly generated JUnit.xml file is always invalid. Could we have a process which guarantees that the JUnit.xml file can only be written completely?

A delayed output of the file at the end of application is fmho not a reasonable option (see #27 (comment)). I can try to delete the file, if it could not be written completely in case of an error. Please, let me know your thoughts.

My proposition to store the XML output in an optional variable and to write it to a file after running all tests could solve this issue.

Adding output to JUnit.xml with JUnit schema files.

33f5c00

QGhd marked this pull request as draft July 31, 2023 11:24

QGhd marked this pull request as ready for review July 31, 2023 11:26

QGhd mentioned this pull request Jul 31, 2023

Test output as JUnit.xml. #26

Closed

jvdp1 reviewed Aug 1, 2023

View reviewed changes

src/testdrive.F90 Outdated Show resolved Hide resolved

jvdp1 reviewed Aug 1, 2023

View reviewed changes

src/testdrive.F90 Outdated Show resolved Hide resolved

jvdp1 reviewed Aug 1, 2023

View reviewed changes

src/testdrive.F90 Outdated Show resolved Hide resolved

jvdp1 reviewed Aug 1, 2023

View reviewed changes

Improvements according comments.

75703cc

awvwgk reviewed Aug 2, 2023

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

awvwgk reviewed Aug 2, 2023

View reviewed changes

awvwgk requested changes Aug 2, 2023

View reviewed changes

Revert .gitignore.

e80c763

QGhd requested review from awvwgk and jvdp1 August 3, 2023 10:10

jvdp1 reviewed Aug 3, 2023

View reviewed changes

QGhd requested a review from jvdp1 August 6, 2023 12:32

jvdp1 reviewed Aug 8, 2023

View reviewed changes

src/testdrive.F90 Outdated Show resolved Hide resolved

jvdp1 reviewed Aug 8, 2023

View reviewed changes

Update src/testdrive.F90

2cae7b8

Variable declaration changed according reviewer's suggestion. Co-authored-by: Jeremie Vandenplas <[email protected]>

jvdp1 mentioned this pull request Aug 13, 2024

Support of JUnit.xml #35

Closed

1 task

awvwgk mentioned this pull request Sep 7, 2024

Add support for JUnit.xml #42

Merged

3 tasks

awvwgk closed this in #42 Sep 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding output to JUnit.xml with JUnit schema files. #27

Adding output to JUnit.xml with JUnit schema files. #27

QGhd commented Jul 31, 2023

jvdp1 commented Aug 1, 2023

jvdp1 Aug 1, 2023 •

edited

Loading

QGhd Aug 1, 2023

QGhd Sep 4, 2023

awvwgk Aug 2, 2023

QGhd Aug 2, 2023

QGhd Aug 5, 2023

QGhd Sep 4, 2023

awvwgk Aug 2, 2023

QGhd Aug 2, 2023

QGhd Aug 5, 2023

QGhd Sep 4, 2023

awvwgk Aug 2, 2023

QGhd Aug 2, 2023

QGhd Aug 5, 2023

QGhd Sep 4, 2023

awvwgk left a comment

QGhd commented Aug 2, 2023 •

edited

Loading

jvdp1 Aug 3, 2023

QGhd Sep 4, 2023

jvdp1 Sep 5, 2023

jvdp1 Aug 3, 2023

QGhd Aug 4, 2023

jvdp1 Aug 8, 2023

QGhd Sep 4, 2023 •

edited

Loading

jvdp1 Sep 5, 2023

jvdp1 Aug 8, 2023

QGhd Sep 4, 2023

QGhd commented Sep 4, 2023

jvdp1 commented Sep 5, 2023

Adding output to JUnit.xml with JUnit schema files. #27

Adding output to JUnit.xml with JUnit schema files. #27

Conversation

QGhd commented Jul 31, 2023

jvdp1 commented Aug 1, 2023

jvdp1 Aug 1, 2023 • edited Loading

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

awvwgk left a comment

Choose a reason for hiding this comment

QGhd commented Aug 2, 2023 • edited Loading

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

QGhd Sep 4, 2023 • edited Loading

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

QGhd commented Sep 4, 2023

jvdp1 commented Sep 5, 2023

jvdp1 Aug 1, 2023 •

edited

Loading

QGhd commented Aug 2, 2023 •

edited

Loading

QGhd Sep 4, 2023 •

edited

Loading