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

Added support for dotnet nunit #206

Closed
wants to merge 2 commits into from

Conversation

simensan
Copy link

@simensan simensan commented Nov 6, 2022

Added support for NUnit 3.0 result xml, which is what Unity Tes Runner produces.

  • Since it has nested test suites I added identation level to TestSuiteResult so it can be formated nicely.
  • Moved sorting of test suites to each parser, since sorting didnt make sense when having nested suites

Any feedback is appreciated!

@dharmendrasha
Copy link
Collaborator

Closing this Because test were not successful.

@simensan
Copy link
Author

The failure of the check seems to be unrelated to tests not being successful?
The output shows: "Tests: 25 passed, 25 total"
"Error: HttpError: Resource not accessible by integration"
Never seen that error before,

@dorny dorny reopened this Nov 30, 2022
@dorny
Copy link
Owner

dorny commented Nov 30, 2022

Thanks @simensan for the PR. You are right. This is the same situation as we have in #208. One of the check run fails due to permission issues. This happens when PR is from forked repo. I have to rework the workflow.

Anyway I see there are some other modification apart from the new NUnit parser. I will review it in the evening after my work day.

@longwuyuan
Copy link

Hi @dorny , I experienced the same message here https://github.com/kubernetes/ingress-nginx/actions/runs/3610316594/jobs/6084083732

Any comments if they are related and a issue pending resolution or if I have a problem in my config

...
Run dorny/test-reporter@v1
  with:
    name: JEST Tests
    path: test/junitreports/*.xml
    reporter: jest-junit
    path-replace-backslashes: false
    list-suites: all
    list-tests: all
    max-annotations: 10
    fail-on-error: true
    only-summary: false
    token: ***
...

I don't have a "artifact" field because my attempts to specify a regex pattern are failing for the "artifact" field. Example I tried /*.xml/

@MostefaKamalLala
Copy link

@simensan gj! @dorny will you have the chance to review this PR any time soon? The workaround using the xslt transformer (from nunit to junit) doesnt produce good results.

@dorny
Copy link
Owner

dorny commented Dec 15, 2022

Sorry guys, I was again quite busy recently and forgot about it. I hope I will make it tomorrow or over the weekend.

@dorny
Copy link
Owner

dorny commented Dec 16, 2022

So I've finally looked into this. It will need some adjustments but I'm definitely for adding NUnit support.
Issues I would like to address before we merge it:

  • There are two suites with the same name "MockTestFixture". In the XML there is also fullname attribute, but this is currently not visible in the report. In the report, you don't know which suite it is until you check individual tests inside.
  • All suites in the table show NaNms time
  • JUnit report seems to be broken due to a change in the sorting.
  • Can there be more than one top level suite? Currently there is the mock-assembly.dll and everything is nested there. So I'm not sure if it's good to have all intended in this case.

@timcassell
Copy link

@simensan Are you planning to address @dorny's feedback and continue work on this?

@jozefizso
Copy link
Collaborator

If the #225 does not address the concerns reported here, we can build upon it in the next release.

@jozefizso jozefizso closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants