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

Truth8 java 8 extension part of the main library? #746

Closed
apflieger opened this issue Aug 27, 2020 · 6 comments
Closed

Truth8 java 8 extension part of the main library? #746

apflieger opened this issue Aug 27, 2020 · 6 comments
Assignees
Labels
P2 has an ETA type=enhancement Make an existing feature better

Comments

@apflieger
Copy link

Hello,

I couldn't find any discussion about that so I wanted to give it a chance.
java 8 is now 6 years old, I doubt there are many legacy project (<java 8) that introduce Truth in their code base whereas people starts new project on java 11 at least now.

What do you think?

@cpovirk
Copy link
Member

cpovirk commented Aug 27, 2020

Thanks. We plan to at least merge the 2 artifacts into 1 "soon" (probably months from now if it's left up to me, but this may be the kind of thing that someone could put together a PR for). We will still keep Truth and Truth8 as separate classes for the moment, but maybe we can convince ourselves that it's safe enough to merge the classes, too. (That might well break something, for Java 7 or for Android. We'd have to see. We probably won't drop compatibility with old versions of Android yet, but if Android works but Java 7 does not, we could consider dropping Java 7.)

@cpovirk cpovirk added P3 not scheduled type=enhancement Make an existing feature better labels Aug 27, 2020
@apflieger
Copy link
Author

Yeah I mean merging Truth and Truth8 because that's what is painful to use.

@cpovirk
Copy link
Member

cpovirk commented Aug 27, 2020

Yeah. I forget if we've experimented to see what might break. Someday :(

copybara-service bot pushed a commit that referenced this issue Aug 1, 2021
…ately.

This is a small step in the general direction of #746

RELNOTES=Truth is now built with `-source 8 -target 8`. This means that it no longer runs under Java 7 VMs. It continues to run under Android, even old versions, for all apps that have [enabled support for Java 8 language features](https://developer.android.com/studio/write/java8-support#supported_features).
PiperOrigin-RevId: 387845958
copybara-service bot pushed a commit that referenced this issue Aug 1, 2021
…ately.

This is a small step in the general direction of #746

RELNOTES=Truth is now built with `-source 8 -target 8`. This means that it no longer runs under Java 7 VMs. It continues to run under Android, even old versions, for all apps that have [enabled support for Java 8 language features](https://developer.android.com/studio/write/java8-support#supported_features).
PiperOrigin-RevId: 387845958
copybara-service bot pushed a commit that referenced this issue Aug 1, 2021
…ately.

This is a small step in the general direction of #746

RELNOTES=Truth is now built with `-source 8 -target 8`. This means that it no longer runs under Java 7 VMs. It continues to run under Android, even old versions, for all apps that have [enabled support for Java 8 language features](https://developer.android.com/studio/write/java8-support#supported_features).
PiperOrigin-RevId: 387845958
copybara-service bot pushed a commit that referenced this issue Aug 2, 2021
…ately.

This is a small step in the general direction of #746

While there, improve the documentation of deprecated `LiteProtoSubject.isEqualTo(MessageLite.Builder)` and `isNotEqualTo(MessageLite.Builder)`, and add a TODO about possible future steps.

RELNOTES=Truth is now built with `-source 8 -target 8`. This means that it no longer runs under Java 7 VMs. It continues to run under Android, even old versions, for all apps that have [enabled support for Java 8 language features](https://developer.android.com/studio/write/java8-support#supported_features).
PiperOrigin-RevId: 387845958
copybara-service bot pushed a commit that referenced this issue Aug 2, 2021
…ately.

This is a small step in the general direction of #746

While there, improve the documentation of deprecated `LiteProtoSubject.isEqualTo(MessageLite.Builder)` and `isNotEqualTo(MessageLite.Builder)`, and add a TODO about possible future steps.

RELNOTES=Truth is now built with `-source 8 -target 8`. This means that it no longer runs under Java 7 VMs. It continues to run under Android, even old versions, for all apps that have [enabled support for Java 8 language features](https://developer.android.com/studio/write/java8-support#supported_features).
PiperOrigin-RevId: 388266403
@bjmi
Copy link

bjmi commented Jan 25, 2022

com.google.common.truth.Truth and com.google.common.truth.Truth8 classes reside in the same package but in different modules. After converting both maven artifacts to OSGI bundles I get java.lang.NoClassDefFoundError: com/google/common/truth/Subject
by calling Truth8.assertThat(Optional<?>).hasValue(expected) in a downstream test bundle.
Both bundles provide Export-Package: com.google.common.truth in MANIFEST.MF. This looks like split package problem. Truth Subject from "main" bundle is not visible in the "extension" bundle.

At the first glance, there are two possible solutions:

  • Move Truth8 to a subpackage like com.google.common.truth.extensions.truth8 like ProtoTruth and Re2jSubjects
  • Merge Truth8 extension with Truth main module (preferred)

@cpovirk
Copy link
Member

cpovirk commented Jan 25, 2022

All true, thanks. We intend to merge them, but it's been difficult to schedule time for Truth among our other priorities. It is frequently on my mind, but that doesn't actually help people who are hitting this issue :(

@cpovirk cpovirk self-assigned this Jan 10, 2024
@cpovirk cpovirk added P2 has an ETA and removed P3 not scheduled labels Jan 10, 2024
copybara-service bot pushed a commit that referenced this issue Jan 10, 2024
While there, make some minor changes to prepare for moving the `Truth8` methods into `Truth`.

This is the first step of #746.

RELNOTES=Moved the `truth-java8-extension` classes into the main `truth` artifact. There is no longer any need to depend on `truth-java8-extension`, which is now empty. (We've also removed the `Truth8` [GWT](https://www.gwtproject.org/) module.)
PiperOrigin-RevId: 597038300
copybara-service bot pushed a commit that referenced this issue Jan 10, 2024
While there, make some minor changes to prepare for moving the `Truth8` methods into `Truth`.

This is the first step of #746.

RELNOTES=Moved the `truth-java8-extension` classes into the main `truth` artifact. There is no longer any need to depend on `truth-java8-extension`, which is now empty. (We've also removed the `Truth8` [GWT](https://www.gwtproject.org/) module.)
PiperOrigin-RevId: 597243015
copybara-service bot pushed a commit that referenced this issue Jan 15, 2024
At that point, some calls to `assertThat(Object)` will become calls to the new overload. That would be a problem if:

- they call `isEqualTo` or `isNotEqualTo`, which we've turned into throwing `@DoNotCall` methods
- they have already collected the `Stream` _or_ they want to operate on the `Stream` afterward

This CL makes `isEqualTo` and `isNotEqualTo` "work" (though it leaves them deprecated, since they're still a bad idea), and it avoids collecting the `Stream` until necessary.

The `isEqualTo` change requires some weird plumbing because I made its failure message retain our warning about `Stream.equals`. That requires creating a new `StreamSubject` with a different `FailureMetadata` instance, which isn't the kind of thing we normally do. (We've done something similar in `ThrowableSubject`, but it is simpler because (a) `ThrowableSubject` uses the "normal" (i.e.., non-no-arg) `check` and (b) `ThrowableSubject` doesn't have to worry about avoiding re-collecting a `Stream`.)

We may want to clamp back down on `isEqualTo` in the future. I've set myself a calendar reminder for mid-year. For now, I just want to make `assertThat(Stream)`, which will already be mildly disruptive, from being even more disruptive.

(progress toward #746)

RELNOTES=Made `StreamSubject` avoid collecting the `Stream` until necessary, and made its `isEqualTo` and `isNotEqualTo` methods no longer always throw.
PiperOrigin-RevId: 598241143
copybara-service bot pushed a commit that referenced this issue Jan 15, 2024
At that point, some calls to `assertThat(Object)` will become calls to the new overload. That would be a problem if:

- they call `isEqualTo` or `isNotEqualTo`, which we've turned into throwing `@DoNotCall` methods
- they have already collected the `Stream` _or_ they want to operate on the `Stream` afterward

This CL makes `isEqualTo` and `isNotEqualTo` "work" (though it leaves them deprecated, since they're still a bad idea), and it avoids collecting the `Stream` until necessary.

The `isEqualTo` change requires some weird plumbing because I made its failure message retain our warning about `Stream.equals`. That requires creating a new `StreamSubject` with a different `FailureMetadata` instance, which isn't the kind of thing we normally do. (We've done something similar in `ThrowableSubject`, but it is simpler because (a) `ThrowableSubject` uses the "normal" (i.e.., non-no-arg) `check` and (b) `ThrowableSubject` doesn't have to worry about avoiding re-collecting a `Stream`.)

We may want to clamp back down on `isEqualTo` in the future. I've set myself a calendar reminder for mid-year. For now, I just want to make `assertThat(Stream)`, which will already be mildly disruptive, from being even more disruptive.

(progress toward #746)

RELNOTES=Made `StreamSubject` avoid collecting the `Stream` until necessary, and made its `isEqualTo` and `isNotEqualTo` methods no longer always throw.
PiperOrigin-RevId: 598607794
copybara-service bot pushed a commit that referenced this issue Jan 15, 2024
…tWithMessage(...).that(optional).isPresent()`, etc., including for `Stream` as well as `Optional`.

That is, you no longer need to use `about(optionals())` (or `about(streams())`). (Later, we'll do the same thing for lesser-used APIs like `optionalInts().)

This CL does _not_ make it possible to write `Truth.assertThat(optional).isPresent()`: For that, you still need to use `Truth8`. A future CL will eliminate the need to use `Truth8`.

This continues our work on #746.

RELNOTES=Added `that` overloads to make it possible to write type-specific assertions when using `expect.that(optional)` and `expect.that(stream)`.
PiperOrigin-RevId: 597240108
copybara-service bot pushed a commit that referenced this issue Jan 15, 2024
…tWithMessage(...).that(optional).isPresent()`, etc., including for `Stream` as well as `Optional`.

That is, you no longer need to use `about(optionals())` (or `about(streams())`). (Later, we'll do the same thing for lesser-used APIs like `optionalInts().)

This CL does _not_ make it possible to write `Truth.assertThat(optional).isPresent()`: For that, you still need to use `Truth8`. A future CL will eliminate the need to use `Truth8`.

This continues our work on #746.

RELNOTES=Added `that` overloads to make it possible to write type-specific assertions when using `expect.that(optional)` and `expect.that(stream)`.
PiperOrigin-RevId: 598637400
copybara-service bot pushed a commit that referenced this issue Jan 15, 2024
… main `Truth` class.

(and prepare AutoValue tests for the new overloads, since the Eclipse compiler considers the (temporary!) signatures of `assertThat(Optional)` to be ambiguous)

We'll post some migration suggestions as part of the release notes.

This is the biggest part of #746.

RELNOTES=Added `assertThat` overloads for `Optional` and `Stream` to the main `Truth` class.
PiperOrigin-RevId: 596422911
copybara-service bot pushed a commit that referenced this issue Feb 2, 2024
…he main `Truth` class.

(split off from cl/603361985 because I was waiting on fixes to a couple projects)

We'll post some migration suggestions as part of the release notes.

This is one of the remaining loose ends of #746.

RELNOTES=Added `Truth.assertThat(Path)` and `(OptionalLong)` overloads (copied from `Truth8`).
PiperOrigin-RevId: 603474032
copybara-service bot pushed a commit that referenced this issue Feb 2, 2024
…he main `Truth` class.

(split off from cl/603361985 because I was waiting on fixes to a couple projects)

We'll post some migration suggestions as part of the release notes.

This is one of the remaining loose ends of #746.

RELNOTES=Added `Truth.assertThat(Path)` and `(OptionalLong)` overloads (copied from `Truth8`).
PiperOrigin-RevId: 603713061
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Feb 2, 2024
This prepares for some migrations off `Truth8` as part of
google/truth#746.

Notably, after this commit, any Gerrit code that compiles in Google's
monorepo should also compile in the Gerrit Git repo. (The reverse is not
yet universally true, but I think it is true as long as Gerrit continues
to avoid static imports from `Truth8`.)

Change-Id: Iefd3f4b2bdc233aeaa75b66f88d529dad712e0db
copybara-service bot pushed a commit that referenced this issue Feb 6, 2024
This will break most users who static import both `Truth.assertThat` and `Truth8.assertThat`. The fix is usually as simple as replacing every reference to `Truth8` with a reference to `Truth`. But we'll post some additional migration information as part of the release notes, as we've already done for [1.3.0](https://github.com/google/truth/releases/tag/v1.3.0) and [1.4.0](https://github.com/google/truth/releases/tag/v1.4.0).

(The type parameters existed to avoid that static import conflict. However, the type parameters also _cause other static import conflicts_, so we don't want them in place in the long term.)

This is one of the remaining loose ends of #746.

RELNOTES=Removed temporary type parameters from `Truth.assertThat(Stream)` and `Truth.assertThat(Optional)`. This can create build errors, which you can fix by replacing all your references to `Truth8` with references to `Truth`.
PiperOrigin-RevId: 602078126
copybara-service bot pushed a commit that referenced this issue Feb 6, 2024
This will break most users who static import both `Truth.assertThat` and `Truth8.assertThat`. The fix is usually as simple as replacing every reference to `Truth8` with a reference to `Truth`. But we'll post some additional migration information as part of the release notes, as we've already done for [1.3.0](https://github.com/google/truth/releases/tag/v1.3.0) and [1.4.0](https://github.com/google/truth/releases/tag/v1.4.0).

(The type parameters existed to avoid that static import conflict. However, the type parameters also _cause other static import conflicts_, so we don't want them in place in the long term.)

This is one of the remaining loose ends of #746.

RELNOTES=Removed temporary type parameters from `Truth.assertThat(Stream)` and `Truth.assertThat(Optional)`. This can create build errors, which you can fix by replacing all your references to `Truth8` with references to `Truth`.
PiperOrigin-RevId: 602078126
copybara-service bot pushed a commit that referenced this issue Feb 6, 2024
This will break most users who static import both `Truth.assertThat` and `Truth8.assertThat`. The fix is usually as simple as replacing every reference to `Truth8` with a reference to `Truth`. But we'll post some additional migration information as part of the release notes, as we've already done for [1.3.0](https://github.com/google/truth/releases/tag/v1.3.0) and [1.4.0](https://github.com/google/truth/releases/tag/v1.4.0).

(The type parameters existed to avoid that static import conflict. However, the type parameters also _cause other static import conflicts_, so we don't want them in place in the long term.)

This is one of the remaining loose ends of #746.

RELNOTES=Removed temporary type parameters from `Truth.assertThat(Stream)` and `Truth.assertThat(Optional)`. This can create build errors, which you can fix by replacing all your references to `Truth8` with references to `Truth`.
PiperOrigin-RevId: 604754613
copybara-service bot pushed a commit that referenced this issue Feb 6, 2024
… main `Truth` class.

This includes deprecating the old class.

This continues work on #746.

RELNOTES=Deprecated `Truth8`. All its functionality is now supported through the main `Truth` API.
PiperOrigin-RevId: 603756994
copybara-service bot pushed a commit that referenced this issue Feb 6, 2024
… main `Truth` class.

This includes deprecating the old class.

This continues work on #746.

RELNOTES=Deprecated `Truth8`. All its functionality is now supported through the main `Truth` API.
PiperOrigin-RevId: 603756994
copybara-service bot pushed a commit that referenced this issue Feb 6, 2024
… main `Truth` class.

This includes deprecating the old class.

This continues work on #746.

RELNOTES=Deprecated `Truth8`. All its functionality is now supported through the main `Truth` API.
PiperOrigin-RevId: 604762265
copybara-service bot pushed a commit that referenced this issue Feb 6, 2024
… main `Truth` class.

This includes deprecating the old class.

This continues work on #746.

RELNOTES=Deprecated `Truth8`. All its functionality is now supported through the main `Truth` API.
PiperOrigin-RevId: 604762265
qtprojectorg pushed a commit to qtqa/gerrit that referenced this issue Feb 8, 2024
This prepares for some migrations off `Truth8` as part of
google/truth#746.

Notably, after this commit, any Gerrit code that compiles in Google's
monorepo should also compile in the Gerrit Git repo. (The reverse is not
yet universally true, but I think it is true as long as Gerrit continues
to avoid static imports from `Truth8`.)

Change-Id: Iefd3f4b2bdc233aeaa75b66f88d529dad712e0db
Release-Notes: skip
copybara-service bot pushed a commit that referenced this issue Feb 15, 2024
This continues our work on #746.

RELNOTES=n/a
PiperOrigin-RevId: 607454835
copybara-service bot pushed a commit that referenced this issue Feb 15, 2024
This continues our work on #746.

RELNOTES=n/a
PiperOrigin-RevId: 607454835
copybara-service bot pushed a commit that referenced this issue Feb 15, 2024
This continues our work on #746.

RELNOTES=n/a
PiperOrigin-RevId: 607468197
copybara-service bot pushed a commit that referenced this issue Feb 15, 2024
This continues our work on #746.

RELNOTES=n/a
PiperOrigin-RevId: 607468197
cpovirk added a commit that referenced this issue Feb 16, 2024
… main `Truth` class.

This includes deprecating the old class.

This continues work on #746.

RELNOTES=Deprecated `Truth8`. All its functionality is now supported through the main `Truth` API.
PiperOrigin-RevId: 604762265
cpovirk added a commit that referenced this issue Feb 16, 2024
This continues our work on #746.

RELNOTES=n/a
PiperOrigin-RevId: 607468197
@cpovirk
Copy link
Member

cpovirk commented Feb 29, 2024

With the release of Truth 1.4.2, this is essentially done: You no longer need the truth-java8-extension artifact, and you no longer need the Truth8 class.

We could still do some more internal cleanup (like to move the tests of the Truth8 functionality into the core directory). But from the perspective of users, this is finally done.

@cpovirk cpovirk closed this as completed Feb 29, 2024
copybara-service bot pushed a commit that referenced this issue Feb 29, 2024
This is an internal loose end in #746.

RELNOTES=n/a
PiperOrigin-RevId: 611587332
copybara-service bot pushed a commit that referenced this issue Feb 29, 2024
This is an internal loose end in #746.

RELNOTES=n/a
PiperOrigin-RevId: 611600975
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Mar 8, 2024
This upgrade makes it possible to use static imports for
`assertThat(Optional)` methods again, so I'm also reverting:

- 454e167
- 177450d

In order to do that, I had to make a change to your `OptionalSubject`.
If you'd prefer, I can keep your `OptionalSubject` as it is, and you can
continue to use the qualified calls (`OptionalSubject.assertThat(...)`)
that I introduced in the earlier commits.

I'm also finishing up some changes that were started in a few files in
e463a3f: I am changing call sites from
`Truth8.assertThat` to `assertThat` (static imported from `Truth`).
(There is one exception: In `ApprovalCopierIT`, I have to use a
qualified call site because another `assertThat` method is declared in
scope, so it would take priority. I at least changed it from
the deprecated `Truth8.assertThat` to `Truth.assertThat`.)

This ties up one of the loose ends left after
google/truth#746.

My hope is that the only remaining loose ends are in
plugins/code_owners:

- cbed17a9a891822d617f3ed1ce59fd7599fe2c8b
- 3f2a11b5a25774b30b7094108343578746929256

Release-Notes: skip
Change-Id: Ic6c941adaf9ae3007a9dd52ecba76de15798cf40
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Mar 13, 2024
This upgrade makes it possible to use static imports for
`assertThat(Optional)` methods again, so I'm also reverting:

- 454e167
- 177450d

In order to do that, I had to make a change to your `OptionalSubject`.
If you'd prefer, I can keep your `OptionalSubject` as it is, and you can
continue to use the qualified calls (`OptionalSubject.assertThat(...)`)
that I introduced in the earlier commits.

I'm also finishing up some changes that were started in a few files in
e463a3f: I am changing call sites from
`Truth8.assertThat` to `assertThat` (static imported from `Truth`).
(There is one exception: In `ApprovalCopierIT`, I have to use a
qualified call site because another `assertThat` method is declared in
scope, so it would take priority. I at least changed it from
the deprecated `Truth8.assertThat` to `Truth.assertThat`.)

This ties up one of the loose ends left after
google/truth#746.

My hope is that the only remaining loose ends are in
plugins/code_owners:

- cbed17a9a891822d617f3ed1ce59fd7599fe2c8b
- 3f2a11b5a25774b30b7094108343578746929256

Release-Notes: skip
Change-Id: Ic6c941adaf9ae3007a9dd52ecba76de15798cf40
lucamilanesio pushed a commit to GerritCodeReview/plugins_code-owners that referenced this issue Mar 14, 2024
As of Truth 1.4.2, we can use static imports again.

This largely reverts commits cbed17a
and 3f2a11b.

This is the last Gerrit-related loose end that I know of from
google/truth#746, aside from the deprecated
automerger plugin:
https://gerrit-review.git.corp.google.com/c/plugins/automerger/+/406837

Change-Id: I9dda0e4f8825e1e635f07c786dba0951414d9eb4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 has an ETA type=enhancement Make an existing feature better
Projects
None yet
Development

No branches or pull requests

3 participants