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

Arb.list fails within edge cases in case of null values #3981

Open
oliverblaha opened this issue Apr 25, 2024 · 5 comments
Open

Arb.list fails within edge cases in case of null values #3981

oliverblaha opened this issue Apr 25, 2024 · 5 comments
Milestone

Comments

@oliverblaha
Copy link
Contributor

Test case: Arb.list(Arb.constant(null), 1..100).edgecases().forAll { }

Expected: Passes and generates lists of null with a variable length.
Actual: java.util.NoSuchElementException: Collection is empty. in collections.kt, Line 108.

Note: This passes if the lower boundary is set to 0.

Currently, edge cases returning null will lead to the conclusion "there is no edge case for this arb" and subsequently fall back to use a random value. If this is null, too, then the particular edge case for the list is ignored. However, this is not only incorrect (either the edge case or the random value might actually be null), but causes no edge case to be left; therefore the random selection of edge cases fails with "Collection is empty."

I'll submit a PR to address this.

github-merge-queue bot pushed a commit that referenced this issue May 9, 2024
This shall fix the issue as described in
#3981.
Instead of just assuming that a null value means "there is no edge
case", this change adds explicitly checking that, effectively fixing the
problem.
@oliverblaha
Copy link
Contributor Author

Fixed in 5.9.0.

@Kantis Kantis reopened this May 18, 2024
@Kantis Kantis added this to the 6.0 milestone May 18, 2024
@Kantis
Copy link
Member

Kantis commented May 18, 2024

Change being reverted due to performance impact. In 6.0 we will change the Arb.edgecase(rs: RandomSource) function to return a Sample<A>? which lets us handle the nested nullability problem (null can currently symbolize both the absence of an edgecase, and the null edgecase)

@oliverblaha
Copy link
Contributor Author

That's a pity, as this will cause some tests to break again :(
Do you have any example that illustrates the performance impact?

@oliverblaha
Copy link
Contributor Author

oliverblaha commented May 20, 2024

Ah - found the reported issue; I like addressing the nullability issue. However, how about going back to previous behavior, but with a workaround for the failing test issue?

What happened was: if the result was null, it would be treated as "there is no edge case", leading to an index exception.
We could relatively easily work around that: if the list of edge cases is empty (which then would cause the exception), just return a list with a single null value. Not perfect, but beats throwing an exception.

(Edit: obviously this shall be a temporary workaround until 6.0, assuming the latter will still take a while.)

@Kantis
Copy link
Member

Kantis commented May 20, 2024

Could you draft a PR to show what you mean? I'll create a separate issue for addressing nullability

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants