Skip to content

[Request] AfterContraint to support more readable usage #1837

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

Closed
mbendtsen opened this issue Oct 12, 2016 · 11 comments
Closed

[Request] AfterContraint to support more readable usage #1837

mbendtsen opened this issue Oct 12, 2016 · 11 comments

Comments

@mbendtsen
Copy link

mbendtsen commented Oct 12, 2016

As a developer
I want to be able to use the AfterConstraint in a more readable and DSL like fasion
so that anyone else that reads my code, quickly understands what the numbers means.

Background
Today when using the After constraint you have to define the delay in milliseconds. If I then want to wait for 3 minutes I can do one of the two:
Specify the delay in milliseconds:
Assert.That(checkStatusFunc, Is.True.After(180000));
Our use a TimeSpan:
Assert.That(checkStatusFunc, Is.True.After((int) TimeSpan.FromMinutes(3).TotalMilliseconds));

None of them is easy to read.
Suggestion, that looks the same as you can do with the WithinConstraint.
Assert.That(checkStatusFunc, Is.True.After(3).Minutes);

If you want to go further you could add the polling interval in the same manner.
Assert.That(checkStatusFunc, Is.True.After(3).Minutes.ByPollingEvery(1).Seconds);

@CharliePoole
Copy link
Member

It's an elegant solution to what I imagine to be a rather rare problem. :-) Or I hope so anyway.

A unit test waiting for three minutes sounds like a bad test. However, I can see it if you use NUnit for functional testing.

As an old programmer, I always considered millisecond timing as a great convenience over counting ticks, but times have changed. :-)

Anyway, it seems like a feature we could include, albeit at low priority.

@mbendtsen
Copy link
Author

Agree, I actually strive not to have any async operation in my unit test, but some times it is needed. And even then I tend to write 2000 for two seconds, because timing issues, like different hardware and such, should not result in my test failing.
But you are right, this example is for function test, in my example I use NUnit together with Specflow.

@CharliePoole
Copy link
Member

But it's a fun feature to program anyway. :-)

@rvignesh89
Copy link

@CharliePoole, I'd like to give this try. I extensively use nunit for unit and integration tests and wanted to read the code anyway. By solving I hope I can score two birds in a stone :)

@CharliePoole
Copy link
Member

OK! I sent you an invitation to be a contributor, which will allow me to assign this to you so you get credit for it.

@rvignesh89
Copy link

@CharliePoole @rprouse do you also want to add the polling interval as suggested by @mbendtsen?

Assert.That(checkStatusFunc, Is.True.After(3).Minutes.ByPollingEvery(1).Seconds);

This should be simple enough with the Interval class in place now.

@CharliePoole
Copy link
Member

I lean toward using PollEvery or Polling or Poll as being shorter. @rprouse what do you think?

@rprouse
Copy link
Member

rprouse commented Oct 22, 2016

I like PollEvery

@CharliePoole
Copy link
Member

@rvignesh89 You should create a new issue for this. Let's use PollEvery(n).Minutes etc. Best way to do the work is to start a new branch based on the latest master and create a new PR when you are ready.

There are some subtleties here. In the most obvious implementation, Minutes applies to the full delay and Minute to the polling interval. But this could be confusing for users. For example, all the following are potentially legal and would do the same thing:

Assert.That(actual, Is.EqualTo(expected).After(3).Minutes.PollEvery(1).Minute);
Assert.That(actual, Is.EqualTo(expected).After(3).PollEvery(1).Minutes.Minute);
Assert.That(actual, Is.EqualTo(expected).After(3).PollEvery(1).Minute.Minutes);
Assert.That(actual, Is.EqualTo(expected).After(3).Minute.PollEvery(1).Minutes);
Assert.That(actual, Is.EqualTo(expected).After(3).Minutes.Minute.PollEvery(1);
Assert.That(actual, Is.EqualTo(expected).After(3).Minute.Minutes.PollEvery(1);

To make it worse, once you type After(3), intellisense will suggest both the singular and the plural form of each time measure!

It's solvable, but no longer the "easyfix" of this issue. :-)

Let's talk implementation on the new issue if you decide to go ahead.

@rvignesh89
Copy link

@CharliePoole I understand the subtleties of this issue now. Let me see what I can do :)

@CharliePoole
Copy link
Member

Please start by creating the issue. :-) I'll assign both of us to it.

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

No branches or pull requests

4 participants