Skip to content

Add parameters and custom name attributes for XUnit#310

Merged
neparij merged 5 commits intoallure-framework:mainfrom
smolchanovsky:feature/parameters-and-custom-names-for-xunit
Mar 21, 2023
Merged

Add parameters and custom name attributes for XUnit#310
neparij merged 5 commits intoallure-framework:mainfrom
smolchanovsky:feature/parameters-and-custom-names-for-xunit

Conversation

@smolchanovsky
Copy link
Contributor

@smolchanovsky smolchanovsky commented Dec 16, 2022

Context

Description and discussion of the solved problem is here: #309

Checklist

@CLAassistant
Copy link

CLAassistant commented Dec 16, 2022

CLA assistant check
All committers have signed the CLA.

@neparij
Copy link
Contributor

neparij commented Dec 20, 2022

Hi, @smolchanovsky !
Thank you for contributing to Allure!
Could you sign a CLA on this contribution?

  • it would be better to override GetDisplayName(IAttributeInfo factAttribute, string s) from Xunit.Sdk.XunitTestCase within Allure.XUnit.AllureXunitTestCase to provide name and fullname without parameters
  • AllureNameAttribute and AllureFullNameAttribute usage is unnecessarily: AllureXunitAttribute is inherited from FactAttribute and already have an DisplayName parameter. It will be ambiguous for users.

@smolchanovsky
Copy link
Contributor Author

smolchanovsky commented Dec 20, 2022

Hi, @smolchanovsky ! Thank you for contributing to Allure! Could you sign a CLA on this contribution?

  • it would be better to override GetDisplayName(IAttributeInfo factAttribute, string s) from Xunit.Sdk.XunitTestCase within Allure.XUnit.AllureXunitTestCase to provide name and fullname without parameters
  • AllureNameAttribute and AllureFullNameAttribute usage is unnecessarily: AllureXunitAttribute is inherited from FactAttribute and already have an DisplayName parameter. It will be ambiguous for users.

@neparij hi!
You're right, it will be ambiguous for users. Frankly, I forgot about the DisplayName property in FactAttribute :)

But one thing bothers me. I'd like to be able to set different values in name and fullName, so DisplayName is not enough. Maybe add an additional property - FullDisplayName?

PS: Now I have signed the CLA, but status is still "not signed yet" 🤔

@neparij
Copy link
Contributor

neparij commented Dec 20, 2022

@neparij hi! You're right, it will be ambiguous for users. Frankly, I forgot about the DisplayName property in FactAttribute :)

But one thing bothers me. I'd like to be able to set different values in name and fullName, so DisplayName is not enough. Maybe add an additional property - FullDisplayName?

PS: Now I have signed the CLA, but status is still "not signed yet" 🤔

You can use full-qualified name for FullDisplayName. Look at GetUniqueID() method in Allure.XUnit.AllureXunitTestCase

CLA assistant fails with smolchanovsky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).

@smolchanovsky smolchanovsky force-pushed the feature/parameters-and-custom-names-for-xunit branch from 220f01f to a6b79c7 Compare December 20, 2022 09:57
@smolchanovsky
Copy link
Contributor Author

smolchanovsky commented Dec 20, 2022

You can use full-qualified name for FullDisplayName. Look at GetUniqueID() method in Allure.XUnit.AllureXunitTestCase

Ok, I'll take a look at the method. Thanks

CLA job completed successfully now. I found out that the wrong email was specified in commits :)

@neparij neparij added the task:improvement Change that improves some user experience but can't be considered a new feature label Dec 22, 2022
@1Playerr
Copy link
Contributor

1Playerr commented Mar 12, 2023

Hi, @neparij

I've decided to resolve PR comments. I've removed redundant attributes AllureNameAttribute and AllureFullNameAttribute as you suggested and added two methods to build test name and full name. The test's name can be also set via DisplayName.
I've also added the new extension method GetFullFormattedTypeName in TypeExtensions to get more readable argument type name when those arguments are generic. Should I add some unit tests for that? What would be the most appropriate project to place them in?

Add TypeExtensions to get readable generic type argument name.
Add methods to build name and full name for a test.
@1Playerr 1Playerr force-pushed the feature/parameters-and-custom-names-for-xunit branch from e92d52d to df71b0a Compare March 13, 2023 10:54
@neparij neparij merged commit 38394d9 into allure-framework:main Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

task:improvement Change that improves some user experience but can't be considered a new feature theme:xunit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants