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

Require configuration failiure tests? #164

Closed
nipafx opened this issue Jan 21, 2020 · 6 comments
Closed

Require configuration failiure tests? #164

nipafx opened this issue Jan 21, 2020 · 6 comments

Comments

@nipafx
Copy link
Member

nipafx commented Jan 21, 2020

Some extensions have tests that check whether broken configurations are handled as expected (e.g. DefaultLocaleTests). Should all extensions do that? (E.g. DefaulTimeZoneTests, which doesn't at the moment.)

@Bukama
Copy link
Member

Bukama commented Jan 21, 2020

I my opinion it's important that a functioniality checks all of its parts. That's why I tried to check the annotation when I was trying to create the @Issue annotation. So I vote for Yes they should do it

@Bukama
Copy link
Member

Bukama commented Apr 28, 2020

As nobody argues against this I remove the in-discussion label and set up for grabs.

@Bukama
Copy link
Member

Bukama commented Jun 4, 2020

Note: Aside from the opened PR in #272 I would like to create a list of all extension which lack of such tests, so we have an overview before starting (and maybe doing double work).

As I don't have much time at the moment, feel free to have a look at the code and add such a list here - thank you.

@Bukama
Copy link
Member

Bukama commented Jun 6, 2020

If I have not overseen anything this should be the extensions which as of today (don't) check if the configuration is correct:

Checks config:

  • RangeSourceArgumentsProvider
  • DefaultLocaleExtension
  • EnvironmentVariableExtension
  • IssueExtension
  • RepeatFailedTestExtension
  • ReportEntryExtension
  • StdIOExtension
  • SystemPropertyExtension
  • TempDirectoryExtension

Does not check / do they need to?

  • DefaultTimeZoneExtension
  • TimeoutExtension

@Michael1993 , @beatngu13 @NPException Did I oversee anything?

@Bukama
Copy link
Member

Bukama commented Jun 11, 2020

Does not check / do they need to?

  • DefaultTimeZoneExtension
  • TimeoutExtension

For the `DefaultTimeZoneExtension´ @Michael1993 has opened PR #272 .

The other is still up for grabs.

nipafx pushed a commit that referenced this issue Jun 20, 2020
nipafx pushed a commit that referenced this issue Jun 20, 2020
With the update to JUnit 5.5, we can now use the
`InvocationInterceptor` extension point to push test execution onto
a separate thread that we can abandon when the specified timeout runs
out. Somewhat surprisingly, using Jupiter's
`Assertions::assertTimeoutPreemptively` worked really well for that.

The extension now also throws `ExtensionConfigurationException` if
the specified timeout is negative.

Closes: #10 
References: #164
PR: #280
@nipafx
Copy link
Member Author

nipafx commented Jun 20, 2020

Closed by #272 and #280.

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

2 participants