Testing the negative case of have_enqueued_sidekiq_job with any arguments?

[molenick-mov]

I'm trying to test that a job was not enqueued. When I write the test like so, the test passes even though the job was enqueued:

expect(MyJob).not_to have_enqueued_sidekiq_job

If I write it with the specific arguments that it is enqueued with, it will fail. I don't want to specify arguments because if they change, my test will not be valid.

expect(MyJob).not_to have_enqueued_sidekiq_job(50, 'coolstuff')

Specifying anything for each arguments gives the result I want, but I would prefer not to have to explicitly state anything about the value or number of arguments for my test.

expect(MyJob).not_to have_enqueued_sidekiq_job(anything, anything)

I would expect using any_args would work, but it does not work in either case:

expect(MyJob).not_to have_enqueued_sidekiq_job(any_args)
expect(MyJob).not_to have_enqueued_sidekiq_job(any_args, any_args)

What is the correct way to test that a job was not enqueued without having to provide any information about the arguments it might receive?

[cgunther]

I've solved similar problems by asserting on how many jobs were queued, sometimes ensuring no jobs were queued. For example:

expect(MyJob.jobs.size).to eq(0)

[jarthod]

I also hit the same problem and was very surprised, after writing all my green tests with expect(MyJob).not_to have_enqueued_sidekiq_job I decided to double check their robustness by breaking the code and was surprised to see the tests were all passing despite jobs being enqueued.

In the end I used the same hack suggested by @cgunther, except I prefer this version because it gives you the details of enqueued jobs when it fails:

expect(MyJob.jobs).to eq([])

But I believe this is quite misleading and unexpected behavior. If there are good reasons to not support this behavior, it should now be allowed and raise when writting such tests IMO.

[cgunther]

Thinking more on this, I don't think it's specific to the negative case, but rather confusion around what the argument-less have_enqueued_sidekiq_job means.

For example, consider the following:

class MyJob
  include Sidekiq::Job

  def perform(id)
    # ...
  end
end

MyJob.perform_async(1)

expect(MyJob).to have_enqueued_sidekiq_job

Do we expect that to pass or fail?

Currently it fails because it's looking for an argument-less job having been enqueued (ie. MyJob.perform_async()), but in this example, the job had an argument.

Applying the line of thinking from this thread, one might expect it to pass on the premise that at least one job was enqueued, regardless of arguments (since none were provided).

For whatever it's worth, the README does note that it's for specified arguments, but we could expand on that to make it clearer: https://github.com/philostler/rspec-sidekiq/blob/ccce13c71ebfe848bc35ec79fc62f63714f9e3e1/README.md?plain=1#L139-L140

Or add a note about preferring a MyJob.jobs assertion when you're trying to assert no jobs.

Ultimately I think the only way we could make the problematic examples raise is if we inspected the job/worker class's perform method to see if there are any required arguments, and if so, error on a bare call to have_enqueued_sidekiq_job. But if all the arguments are optional, I'd imagine we can't error as we don't know if you're trying to assert a job was/wasn't enqueued generally, or if you're trying to assert a job was/wasn't enqueued with no arguments.

[jarthod]

@cgunther Indeed that example you gave is less common for me (because when I test the presence of a Job I want to make sure it's the right one too so I check arguments) but I also find it unatural.

This is may be habits but most methods I know in the rspec world works this way. For example:

expect { }.to raise_error # does pass if any exception is raised
expect { }.not_to raise_error # does fail if any exception is raised
# And that is the recommended way for less brittle tests

And closer to this one the ActiveJob matcher also works in this same precictable way:

expect { }.to enqueue_job # does pass if any job is enqueued
expect { }.not_to enqueue_job # does fail if any job is enqueued

There are probably more examples in the rspec world, I believe ruby developers are using to this and the principle of least astonishment may prime if the "it's logical to me" argument is not decisive enough.

[cgunther]

It'd probably be a big migration for existing users of this library, but maybe rspec-rails's native have_enqueued_job is a better pattern to follow, where the argument-less have_enqueued_sidekiq_job could simply check for presence or lack of any job (no argument checking), and you'd optionally append with when you want argument-checking (ie. have_enqueued_sidekiq_job.with(1)).

I'm just a fellow user of this library, so it'd probably be up to someone to take a stab at this in a PR.

[jarthod]

Indeed, that's another good example :)

I could take a stab at a PR but I am already spending a lot energy writting PRs and maintaing forks for abandonned projects that I'll only do it with the approval of the maintainer first (so if I know it has a chance to be merged someday).

[wspurgin]

Not that it's exactly solving this use case but #200 would add support for the builtin argument matchers, so you could do something like:

expect(SomeJob).not_to have_enqueue_sidekiq_job(any_args)

It's a little more verbose (and doing any negation math when reading an expectation might still cause confusion), but I think that's probably the best solution for now. I can add a note in the README

[wspurgin]

Piling on, with the new block syntax this works and is a little more intuitive.

expect { "do nothing" }.not_to enqueue_sidekiq_job

[jarthod]

I like this block version indeed :+1: It won't solve the initial issue though, which is the unexpected behavior of the existing syntax. Both in comparison with similar rspec matchers and now also with the block matcher of the same library. So more people will likely fall into this trap in the future but as you prefer of course, 4.0 being a major it could have been a good time to address this.

[wspurgin]

Just because there was a long time between versions, I was aiming to not disrupt existing projects and their use of the existing matchers. In future versions (since I'll be maintaining this going forward), I'll look into making the have_enqueued_sidekiq_job matcher also default to looking for "any args"