Assertj-core: Add overloaded methods with boxed arrays for contains assertions

Created on 30 Nov 2020  ยท  21Comments  ยท  Source: assertj/assertj-core

Summary

Related to #1948, we should add the overloaded methods with boxed arrays for all the primitive array types:

  • [x] AbstractBooleanArrayAssert (84fc616)
  • [x] AbstractByteArrayAssert (6936b6133eec1f6a11e052724e69bd77ed0b19ab)
  • [x] AbstractCharArrayAssert (48794650806b33dbfd8f72f2fc09bf7d92686d0b)
  • [x] AbstractDoubleArrayAssert (dcfdd05dd01030bcd6f6210d4d48455d49c51b52)
  • [x] AbstractFloatArrayAssert (dcfdd05dd01030bcd6f6210d4d48455d49c51b52)
  • [x] AbstractIntArrayAssert (0c5f366dcfd31dee9867ccafbec7dc8a97f5d100)
  • [x] AbstractLongArrayAssert (#2042)
  • [x] AbstractShortArrayAssert (6936b6133eec1f6a11e052724e69bd77ed0b19ab)
good first issue new feature

Most helpful comment

Feedback provided, thanks @mayralgr and @OmarMorales71!

All 21 comments

@scordio Hello again :D, Can I contribute in the AbstractIntArrayAssert? Could you give me more information about it? how I have to add the overloaded methods? I saw the PR for the AbstractLongArrayAssert and I have not seen the difference yet.

Hi @scordio I also want to contribute in this issue, could you give me more information?

Hi @OmarMorales71 and @mayralgr, thank you both!

The target is to add additional contains methods that support boxed arrays of the corresponding primitive type. For example, in case of int arrays we already have:

  public SELF contains(int... values) {
    ...
  }

and the corresponding method to add would be:

  public SELF contains(Integer[] values) {
    ...
  }

All of the new methods would delegate the assertion to the original internal method, just converting the boxed array to a primitive type array.

The one I did for boolean arrays (84fc616) should be a good example for another primitive type.

Just let us know which array type you would like to work on so we prevent overlapping. As usual, let us know in case of any questions, and feel free to raise a PR even in a draft state.

@scordio well first I'm going to take AbstractDoubleArrayAssert, AbstractFloatArrayAssert and AbstractIntArrayAssert. I'm working on it.

Thanks @OmarMorales71, I suggest you raise a PR when you have finished AbstractDoubleArrayAssert to get sone feedback before doing the other ones, the idea is that if you have some rework to do it will only be on the double assertions you will know better for the next ones.

@scordio Perfect!, I'm working on AbstractIntArrayAssert ๐Ÿ‘

@scordio I'll work on AbstractCharArrayAssert first to raise a PR to get feedback

@scordio, the pull request is ready for feedback. Let me know if everything is okay to continue with AbstractByteArrayAssert and AbstractShortArrayAssert.
Also, one check, the CI / OS macOS-latest (pull_request) Failing after 2m โ€” OS macOS-latest is failing, I am not sure why, if you have some info, I'll appreciate it

@mayralgr thanks for the PR, I'll have a look soon. No worries about the test failure, we have a flaky test that has not been fixed yet.

Hi @scordio by any chance have you had the opportunity to check the pr's?

@mayralgr #2070 checked and merged, thank you!

thanks @scordio I'll take in count the feedback to open other PR for the other two ๐Ÿ˜„

@OmarMorales71 #2064 checked and merged, thank you!

@scordio I'll open other PR for the other two, thanks for the feedback ๐Ÿ˜

HI @scordio, I opened a draft https://github.com/assertj/assertj-core/pull/2077 for Byte and Short, I just have a quick question, in the ByteArray assert int values are considered in overloaded methods

public SELF containsAnyOf(int... values) {
    arrays.assertContainsAnyOf(info, actual, arrays.toByteArray(values));
    return myself;
  }

this should also be considered to be overloaded, like

public SELF containsAnyOf(Int[] values) {
    arrays.assertContainsAnyOf(info, actual, arrays.toByteArray(toPrimitiveIntArray(values)));
    return myself;
  }

or should I just limit to the Byte[]?

same in short

Thank you!

@mayralgr I would go with Byte[] only. The integer-based methods were probably introduced to avoid casting on each input literal.

Hi @scordio, the pr is ready, let me know if corrections are needed.
Thanks!

@scordio The PR is ready!

@scordio let us know if something is missing ๐Ÿ˜„

Feedback provided, thanks @mayralgr and @OmarMorales71!

Thank you both @mayralgr and @OmarMorales71!

Was this page helpful?
0 / 5 - 0 ratings