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

Add junit_suite_name ini option #2274

Merged
merged 6 commits into from
May 13, 2017
Merged

Add junit_suite_name ini option #2274

merged 6 commits into from
May 13, 2017

Conversation

dimp-gh
Copy link
Contributor

@dimp-gh dimp-gh commented Feb 22, 2017

It can be used to specify <testsuite>'s name attribute in JUnit XML report. Resolves #533.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 92.731% when pulling c119e43 on dmand:feat/junitxml/suite-name-option into 44ad369 on pytest-dev:features.

@nicoddemus
Copy link
Member

Perhaps it would make more sense as a pytest.ini option? It feels more natural to me to put something like this as a permanent configuration instead of passing all the time in the CLI, and since pytest 3.0 is also possible to override config options in the command line using -o, so even that use case is still supported.

@nicoddemus
Copy link
Member

Could please also add one line or two to the docs? I think this should go into usage.rst, right after introducing the command-line option in the "Creating JUnitXML format files" section.

Creating JUnitXML format files
----------------------------------------------------

To create result files which can be read by Jenkins_ or other Continuous
integration servers, use this invocation::

    pytest --junitxml=path

to create an XML file at ``path``.

.. versionadded:: 3.1

To set the name of the root test suite xml item, you can configure the ``junit_suite_name`` option in your config file:

.. code-block:: ini
    [pytest]
    junit_suite_name = my_suite

CHANGELOG.rst Outdated
@@ -5,6 +5,8 @@
New Features
------------

* junitxml: Add `--junit-suite-name` option to specify root `<testsuite>` name for JUnit XML reports
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a RST file, so you should use double back-ticks to get a monospaced font:

* junitxml: Add ``--junit-suite-name`` option to specify root ``<testsuite>`` name for JUnit XML reports.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, please also mention the original issue (#533) here (take a look at the other entries for see the syntax).

@nicoddemus
Copy link
Member

Hmm strange, I don't see a name attribute for the testsuite tag in the JUnitXML schema, but the current XML generated by pytest works and it uses "pytest" as the test suite name...

@dimp-gh
Copy link
Contributor Author

dimp-gh commented Feb 24, 2017

Pretty much every project i know that generates JUnit-compatible reports uses name attribute of the <testsuite>. It may be a convention, but it's a strong one.

Thanks for the review. My goal is to get familiar with the pytest project, so every comment and suggestion is really helpful.

I agree that this new option belongs to configuration file, not only CLI. I'll update the PR and ping you back.

@nicoddemus
Copy link
Member

Thanks for the review. My goal is to get familiar with the pytest project, so every comment and suggestion is really helpful.

Nice to know. 👍

I agree that this new option belongs to configuration file, not only CLI. I'll update the PR and ping you back.

Great, just in case I wasn't very clear, I think we should have only the configuration option, as opposed to both. Since pytest 3.0 we have the -o option which we can use in the CLI to override configuration options:

pytest -o junit_suite_name=my_suite

Please let us know if you have any questions or need any pointers.

@nicoddemus
Copy link
Member

@dmand gentle ping. 😁

We plan to release 3.1 somewhat soon, so it would be nice to get this in.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, change from CLI argument to ini option

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 92.346% when pulling 757e7b1 on dmand:feat/junitxml/suite-name-option into b6125d9 on pytest-dev:features.

@dimp-gh
Copy link
Contributor Author

dimp-gh commented May 12, 2017

@nicoddemus sorry for the delay. I've updated the PR to use ini option and updated docs/changelog. Let me know if anything needs changes.

@dimp-gh dimp-gh changed the title Add '--junit-suite-name' CLI option Add junit_suite_name ini option May 12, 2017
@dimp-gh dimp-gh changed the title Add junit_suite_name ini option Add junit_suite_name ini option May 12, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 92.346% when pulling 18ea4e9 on dmand:feat/junitxml/suite-name-option into b6125d9 on pytest-dev:features.

@nicoddemus
Copy link
Member

Thanks @dmand for the follow up!

Unless somebody wants more time to review this, I will marge it later.

@RonnyPfannschmidt
Copy link
Member

lovely changes, for nice final history a rebase instead of the merge might be nice,

@nicoddemus
Copy link
Member

Hmm we could have done te rebase directly here in the GUI but we have disabled that option it seems.

Rebasing using the UI right before merging seems very useful. Perhaps we should re-enable that option?

@nicoddemus
Copy link
Member

Rebased and small changes in the tests.

This can be merged as soon as CI passes again. 👍

@RonnyPfannschmidt
Copy link
Member

@nicoddemus i disabled it because i dont want us to lie about the history submitted, in general i would prefer a merge on our side over a rebase done by us

@nicoddemus
Copy link
Member

Well in the end the result is exactly the same: I did end up rebasing it myself and pushed to @dmand's branch anyway. It was just more work in the end.

But I don't really care that much, so if you want to play on the safe side that's fine by me. 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 92.346% when pulling f39f416 on dmand:feat/junitxml/suite-name-option into b6125d9 on pytest-dev:features.

@dimp-gh
Copy link
Contributor Author

dimp-gh commented May 13, 2017

@nicoddemus thanks for taking care of rebasing.

@RonnyPfannschmidt RonnyPfannschmidt merged commit 783670b into pytest-dev:features May 13, 2017
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.

4 participants