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

IterableSubject constructor documentation for check method #634

Open
elharo opened this issue Oct 9, 2019 · 6 comments
Open

IterableSubject constructor documentation for check method #634

elharo opened this issue Oct 9, 2019 · 6 comments
Labels
P3 not scheduled type=api-docs Change/add API documentation

Comments

@elharo
Copy link
Contributor

elharo commented Oct 9, 2019

The API doc for the constructor https://truth.dev/api/1.0/com/google/common/truth/IterableSubject.html#IterableSubject-com.google.common.truth.FailureMetadata-java.lang.Iterable- says "Constructor for use by subclasses. If you want to create an instance of this class itself, call check(...).that(actual)."

However, the check method it links to and the others I can see are protected so I can't call them either.

@netdpb
Copy link
Member

netdpb commented Oct 11, 2019

Right. That's for implementations of Subject. What are you trying to do?

@cpovirk
Copy link
Member

cpovirk commented Oct 11, 2019

(I did figure that someone would call me on this incomplete doc eventually, so I, too, am interested to hear about what brought you here. Thanks for filing the issue.)

@elharo
Copy link
Contributor Author

elharo commented Oct 11, 2019

This came out in GoogleCloudPlatform/cloud-opensource-java#954

I was doing a quick check if a list had changed between versions. To detect the change, I compared the sizes of the lists. I didn't compare the contents because the list was long and I didn't want to reproduce and keep up to date the several hundred elements of the list in the test code. Not perfect, I know, but this allows us to update the test to the new version without updating the previously published version.

If, and only if, the size of the list has changed then I want to see exactly what changed. I was trying to use IterableSubject to compare the two lists to give me a nicely formatted diff. I was not using Truth.assertThat(list).containsExactly() because I can't update that without publishing a new canonical version, and I can't publish a new canonical version until the tests pass. Hence the simple size comparison.

@netdpb
Copy link
Member

netdpb commented Oct 11, 2019

I still don't fully understand why containsExactlyElementsIn() doesn't work for that use case. The test is comparing the actual list to an expected list, but only if the sizes have changed; why not also compare if the sizes have not changed?

It seems to me like whether to use an explicit set of expected elements vs. one read from some other source (as the test is currently doing) is orthogonal to which mechanism it uses to compare the actual set to the expected set.

@elharo
Copy link
Contributor Author

elharo commented Oct 11, 2019

You pointed out the reason a few comments ago. If I take out the check for the size I can't update the test so it passes when the local copy differs from the last published copy.

@netdpb
Copy link
Member

netdpb commented Oct 11, 2019

I think that's a separate issue. Even if you leave the size check in, you can't update the test so it passes when the local copy intentionally adds an element. And if the local copy unintentionally differs but the size is the same, the size check means you'll have a false negative (the test will pass but it shouldn't).

@nick-someone nick-someone added P3 not scheduled type=api-docs Change/add API documentation labels Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 not scheduled type=api-docs Change/add API documentation
Projects
None yet
Development

No branches or pull requests

4 participants