Skip to content

Implement automatic retries using UISettingsAttribute#84

Merged
AArnott merged 4 commits into
AArnott:mainfrom
sharwell:ui-settings
Jun 5, 2023
Merged

Implement automatic retries using UISettingsAttribute#84
AArnott merged 4 commits into
AArnott:mainfrom
sharwell:ui-settings

Conversation

@sharwell

@sharwell sharwell commented Jun 5, 2023

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/Xunit.StaFact/Sdk/UITheoryTestCaseRunner.cs Outdated
Comment thread src/Xunit.StaFact/Sdk/UITestCase.cs Outdated
private static TValue GetNamedArgument<TValue>(IAttributeInfo factAttribute, IAttributeInfo[] settingsAttributes, string argumentName, Func<TValue, bool> isValidValue, Func<TValue, TValue, TValue>? merge, TValue defaultValue)
{
StrongBox<TValue>? result = null;
if (TryGetNamedArgument(factAttribute, argumentName, isValidValue, out TValue? value))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

📝 This was originally designed around having the properties be settable on both the fact attribute (e.g. WinFormsFactAttribute.MaxAttempts) and a separate settings attribute (in this case, UISettingsAttribute). I retained the full logic even though the current implementation only allows the values to be set in one location.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I've dramatically simplified this class to focus on the feature you're adding.

Comment thread src/Xunit.StaFact/Sdk/UITestRunner.cs
@AArnott

AArnott commented Jun 5, 2023

Copy link
Copy Markdown
Owner

Please merge ui-settings from this repo into your fork to update the PR.
The Test Explorer behavior in VS is pretty good at this point, IMO. With my changes tests actually can report as passed and failed (whereas before they always reported skipped regardless).
Azure Pipelines reports each test result (e.g. 'skipped' or 'passed) as an individual test, which isn't great, but I suspect it's as good as we'll get.


namespace Xunit;

public record struct UISettingsKey(int MaxAttempts)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This seems like unnecessary new public API. In fact it seems redundant with the attribute, so I dropped the struct entirely.

private static TValue GetNamedArgument<TValue>(IAttributeInfo factAttribute, IAttributeInfo[] settingsAttributes, string argumentName, Func<TValue, bool> isValidValue, Func<TValue, TValue, TValue>? merge, TValue defaultValue)
{
StrongBox<TValue>? result = null;
if (TryGetNamedArgument(factAttribute, argumentName, isValidValue, out TValue? value))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I've dramatically simplified this class to focus on the feature you're adding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants