Skip to content

GH-2639: Implement skeleton for ClassLoaderAssert - #2645

Merged
scordio merged 4 commits into
assertj:mainfrom
ascopes:feature/2639-classloader-assertions-base
Jul 12, 2022
Merged

GH-2639: Implement skeleton for ClassLoaderAssert#2645
scordio merged 4 commits into
assertj:mainfrom
ascopes:feature/2639-classloader-assertions-base

Conversation

@ascopes

@ascopes ascopes commented Jun 3, 2022

Copy link
Copy Markdown
Contributor

First PR for #2639.

  • Implement skeleton type for AbstractClassLoaderAssert.
  • Implement ClassLoaderAssert implementation.
  • Hook up with Assertions, Assumptions, BDDAssertions, BDDAssumptions,
    BDDSoftAssertionsProvider, InstanceOfAssertFactories,
    StandardSoftAssertionsProvider, WithAssertions, and WithAssumptions.
  • Write ClassLoaderAssert-bound tests for Assertions, Assumptions,
    BDDAssertions, BDDAssumptions, InstanceOfAssertFactories,
    WithAssertions.
  • AssertionsUtil#expectAssumptionNotMetException now returns
    an AssumptionViolatedException rather than void, as to match with
    the API for AssertionsUtil#expectAssertionError. Since nothing
    relied on the return value before as this was a void method, this
    should not break anything existing.

I will add code examples to the second PR once a solid API has
been agreed on, as it prevents missing anything and creating
erroneous documentation.

@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch 4 times, most recently from 4ebafef to 4420781 Compare June 3, 2022 12:54
scordio
scordio reviewed Jun 3, 2022
View reviewed changes
Comment thread src/main/java/org/assertj/core/api/Assertions.java Outdated
@ascopes

ascopes commented Jun 3, 2022

Copy link
Copy Markdown
Contributor Author

I am out for the rest of today and part of tomorrow, so I will likely continue to look at this on Sunday at some point

@scordio

scordio commented Jun 3, 2022

Copy link
Copy Markdown
Member

Sure, take your time @ascopes, and thank you!

@ascopes
ascopes marked this pull request as draft June 4, 2022 07:46
@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch from 27211b4 to f782571 Compare June 4, 2022 11:21
@ascopes
ascopes marked this pull request as ready for review June 4, 2022 11:21
@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch from f782571 to e607f2d Compare June 4, 2022 11:25
@ascopes
ascopes requested a review from scordio June 4, 2022 12:34
@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch from e607f2d to eaf3da2 Compare June 5, 2022 11:02
@ascopes

ascopes commented Jun 5, 2022

Copy link
Copy Markdown
Contributor Author

Added a commit at the start that addresses a transitive issue with the Future tests on the Windows runner. I will cherry-pick this onto a separate branch so it can be reviewed separately first.

@ascopes ascopes mentioned this pull request Jun 5, 2022
Closed
5 tasks done
@scordio

scordio commented Jun 8, 2022

Copy link
Copy Markdown
Member

Thanks @ascopes, I'll tackle this one as soon as I'm done with #2573

@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch from 0ce5b8b to 576fe97 Compare June 15, 2022 07:37
@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch from 576fe97 to 775ee47 Compare June 18, 2022 10:21
@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch from 775ee47 to c8ceb18 Compare June 25, 2022 16:04
ascopes added 2 commits June 27, 2022 08:17
@ascopes
assertjGH-2369: Implement AbstractClassLoaderAssert and ClassLoaderAs…
c101f25 
…sert
@ascopes
assertjGH-2639: Implement skeleton for ClassLoaderAssert
2694870 
- Implement skeleton type for AbstractClassLoaderAssert.
- Implement ClassLoaderAssert implementation.
- Hook up with Assertions, Assumptions, BDDAssertions, BDDAssumptions,
  BDDSoftAssertionsProvider, InstanceOfAssertFactories,
  StandardSoftAssertionsProvider, WithAssertions, and WithAssumptions.
- Write ClassLoaderAssert-bound tests for Assertions, Assumptions,
  BDDAssertions, BDDAssumptions, InstanceOfAssertFactories,
  WithAssertions.
- AssertionsUtil#expectAssumptionNotMetException now returns
  an AssumptionViolatedException rather than void, as to match with
  the API for AssertionsUtil#expectAssertionError. Since nothing
  relied on the return value before as this was a void method, this
  should not break anything existing.
@ascopes
ascopes force-pushed the feature/2639-classloader-assertions-base branch from c8ceb18 to 2694870 Compare June 27, 2022 07:17
@scordio scordio self-assigned this Jul 9, 2022
@scordio scordio added this to the 3.24.0 milestone Jul 9, 2022
@scordio
scordio merged commit ce3ee9b into assertj:main Jul 12, 2022
@scordio

scordio commented Jul 12, 2022

Copy link
Copy Markdown
Member

This is now merged. Thanks, @ascopes!

I applied some changes like adding the version tags and some missing assertThat entry points.

Also, I reverted the changes for expectAssumptionNotMetException - the idea is not wrong, but after some GIVEN / WHEN / THEN refactoring the change was no longer needed and I preferred to avoid another style of testing for assumptions, an area that is already quite heterogeneous.

scordio added a commit that referenced this pull request Dec 23, 2022
@scordio
Revert "Add baseline for ClassLoader assertions (#2645)"
89400f6 
This reverts commit ce3ee9b. These
changes together with the ones in #2650 will be proposed again in a
separate pull request.
scordio added a commit that referenced this pull request Dec 23, 2022
@scordio
Revert "Add baseline for ClassLoader assertions (#2645)" (#2886)
510578d 
This reverts commit ce3ee9b. These
changes together with the ones in #2650 will be proposed again in a
separate pull request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scordio scordio Awaiting requested review from scordio

Assignees

See More

@scordio scordio

Labels

None yet

Projects

None yet

Milestone

3.24.0

Development

Successfully merging this pull request may close these issues.

2 participants

@ascopes @scordio