TCK spec309_requestNegativeNumber may not always issue a negative request

[akarnokd]

Correct me if I'm wrong, but Random.nextInt(N) may return 0, which is not negative thus the TCK test required_spec309_requestNegativeNumberMustSignalIllegalArgumentException may not actually check for a negative value:

  // Verifies rule: https://github.com/reactive-streams/reactive-streams-jvm#3.9
  @Override @Test
  public void required_spec309_requestNegativeNumberMustSignalIllegalArgumentException() throws Throwable {
    activePublisherTest(10, false, new PublisherTestRun<T>() {
      @Override
      public void run(Publisher<T> pub) throws Throwable {
        final ManualSubscriber<T> sub = env.newManualSubscriber(pub);
        final Random r = new Random();
        sub.request(-r.nextInt(Integer.MAX_VALUE)); // <-------------------------------------------------
        sub.expectErrorWithMessage(IllegalArgumentException.class, "3.9"); // we do require implementations to mention the rule number at the very least
      }
    });
  }

Apart from that, is there a particular reason this check has to be random? Why not request some negative constant integer value?

[ktoso]

Yes that's true, please - 1 there if you want to submit a PR for it.

Apart from that, is there a particular reason this check has to be random? Why not request some negative constant integer value?

To exercise weird things, also large numbers. I don't see a problem with keeping the Random.

[akarnokd]

Yes that's true, please - 1 there if you want to submit a PR for it.

Definitely will, but first I wanted to submit a PR about something else which may or may not conflict with changes to the class.

[viktorklang]

I'd rather have a random number to trigger the same failure no matter how "negative" the number is. (also 0 is as illegal as negative)

[ktoso]

I meant subtracting 1 from the random value to make the spec correct in terms of the name.
A separate test for 0 should be added then - likely a good idea anyway, since we want to be sure that is the same handled across impl.