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

Replace Upstream Test Code With Dependency #6

Closed
nipafx opened this issue Nov 25, 2016 · 5 comments
Closed

Replace Upstream Test Code With Dependency #6

nipafx opened this issue Nov 25, 2016 · 5 comments

Comments

@nipafx
Copy link
Member

nipafx commented Nov 25, 2016

#4 enabled tests to be written like in JUnit 5. Part of the implementation copy-pasted a bunch of classes from the project (see f613211 for reasons). As soon as JUnit publishes test artifacts (see junit-team/junit5#572) a proper dependency can be established and the class can be removed.

@nipafx nipafx changed the title Replace Code From Upstream With Dependencie Replace Upstream Test Code With Dependency Dec 23, 2016
@nipafx nipafx added this to the Cleaner Pioneers milestone Jan 7, 2020
@Bukama
Copy link
Member

Bukama commented Mar 29, 2020

As soon as JUnit publishes test artifacts (see junit-team/junit5#572) a proper dependency can be established and the class can be removed.

I asked for an update yesterday and Marc replied as this

Please upgrade to 5.4.2 or later and use junit-platform-testkit.

So we can remove the classes

@nipafx
Copy link
Member Author

nipafx commented Mar 30, 2020

Great, lets do that!

@nipafx
Copy link
Member Author

nipafx commented Mar 30, 2020

And thanks for following up on this! 👍

@nipafx
Copy link
Member Author

nipafx commented Mar 30, 2020

I realize right now that I never considered whether Marc's answer applies to what we specifically are doing with the internal API. Did you give this a try? Maybe we should. :) If you didn't and don't want to, we can do it tomorrow on stream.

nipafx pushed a commit that referenced this issue Mar 31, 2020
In order to remove our use of internal APIs (see #6), we need to
upgrade our dependency on JUnit to at least 5.4.2.

PR: #210
Closes: #209
@nipafx
Copy link
Member Author

nipafx commented Mar 31, 2020

We gave this a try and it looks like the API does everything we need. I'll merge #210 momentarily and then we can get to work on this. I created a proof of concept in issues/6-testkit-api. Let's do it as follows:

  • replace all uses of the internal API with the new one
  • open a PR, so we can have a look and see how we can improve by adding our own functionality
  • write that functionality and refactor accordingly (same branch)
  • another review, then we merge
  • open an issue with JUnit that our API rocks harder than theirs 😁

@Bukama Bukama self-assigned this Mar 31, 2020
@nipafx nipafx closed this as completed in 0b79db8 Apr 24, 2020
Bukama pushed a commit that referenced this issue Jul 14, 2020
In 0b79db8 (#6 / #218), we moved all assertion-like methods onto the
`ExecutionResults` class even though that wasn't a good fit. The
intention was to collect all such methods to then more easily replace
them with proper AssertJ-like assertions that we needed to write
ourselves.

This change implements these methods. The API is pretty good already,
but we expect that after using it for a while the experience with it
as well as new use cases may lead to further changes (see #298).

Closes: #232
PR: #245
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