Skip to content

Bugfix/deprecated markinfo - #295

Merged
sseliverstov merged 5 commits into
allure-framework:masterfrom
Sup3rGeo:bugfix/deprecated-markinfo
Oct 5, 2018
Merged

sseliverstov merged 5 commits into
allure-framework:masterfrom
Sup3rGeo:bugfix/deprecated-markinfo

Conversation

@Sup3rGeo

@Sup3rGeo Sup3rGeo commented Oct 4, 2018 •

Copy link
Copy Markdown
Contributor

//: # (
. Thank you so much for sending us a pull request!
.
. Make sure you have a clear name for your pull request.
. The name should start with a capital letter and no dot is required in the end of the sentence.
. To link the request with isses use the following notation: (fixes #123, fixes #321)
.
. An example of good pull request names:
. - Add Russian translation (fixes #123)
. - Add an ability to disable default plugins
. - Support emoji in test descriptions
)

Context

Fixes #291, #119

Checklist

@sseliverstov

Copy link
Copy Markdown
Contributor

Hi, Victor.

Why do you mark the closest maker? What kind of use case you can't resolve with reason and order of marker?

@Sup3rGeo

Sup3rGeo commented Oct 4, 2018 •

Copy link
Copy Markdown
Contributor Author

Basically from the pytest documentation (https://docs.pytest.org/en/latest/mark.html#updating-code) there can be some cases where you have @pytest.mark.something(class_arg) at the class level, then a @pytest.mark.something(function_specific_arg) on the method level, and the function will actually have both markers, but only the closest one (at the function level) is the active one.

So thats why I marked "closest" to be explicit on that. We could leave it implicit in terms of the order, but when seeing them at the allure report I am not sure if this would be clear enough for users.

What do you think?

@Sup3rGeo

Sup3rGeo commented Oct 4, 2018

Copy link
Copy Markdown
Contributor Author

Another idea: Not all pytest marks are tags in the general sense (for instance, use_fixtures #119, parametrize, etc).
The same argument can be used for
1- Not considering also the other builtin fixtures as allure tags (skip, skipif, xfail).
2- Even further, only considering as tags pytest marks that doesn't have any argument.

I am relatively inclined to implement even n.2. The information about the test being skipped should be probably in another metadata for the allure result.

@sseliverstov

Copy link
Copy Markdown
Contributor

In general, separate pytest marks and user marks is right way. Tag is a common feature for all frameworks and pytest marks isn't suitable there.

BTW, we will improve ui possibilities for tags and I don't sure that filtering for "tag" and "tag closest" would be usable.

@Sup3rGeo

Sup3rGeo commented Oct 4, 2018

Copy link
Copy Markdown
Contributor Author

Good, will update the PR then.

Also opened a discussion in pytest about that (pytest-dev/pytest#4081).

@RonnyPfannschmidt

Copy link
Copy Markdown

the very first iteration of pytest marks from more than a decade ago would fir the concept of "tags" in a very contrived way

the modern version is not matching the concept very well and it may be sensible to think about a path towards correctly matching them

@sseliverstov

Copy link
Copy Markdown
Contributor

@Sup3rGeo, Can you add [WIP] prefix to subject if pr is not ready for review and merging? Did you commit all that you want in this pr?

@Sup3rGeo

Sup3rGeo commented Oct 5, 2018

Copy link
Copy Markdown
Contributor Author

Alright, I will do for next PRs! I believe this one is good to go, if you approve it.

@sseliverstov
sseliverstov merged commit 97ee3c9 into allure-framework:master Oct 5, 2018
@Sup3rGeo
Sup3rGeo deleted the bugfix/deprecated-markinfo branch October 5, 2018 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants