exceed_query_limit expectation should be consistent with skip_cache option

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Close this issue

In !113080 (comment 1299231021) we noticed that N+1 specs can create false positives if the skip_cache option is not consistend in both QUeryRecorder blocks (one created as control and the other provided for the expectation).

We should enforce with an error that if control = ActiveRecord::QueryRecorder.new(skip_cached: false) is used for control, it can only be compared with the matcher exceed_all_query_limit(control). Using exceed_query_limit will skip cache when comparing both query counts.

There are more matchers than exceed_query_limit and exceed_all_query_limit like:

• issue_fewer_queries_than
• issue_same_number_of_queries_as
• match_query_count
• issue_same_number_of_queries_as

We need to fix all of them

Proposed solution

• Modify existing matchers to raise an error if their cache value in QueryRecorder is not consistent
• Create a new legacy matcher that allows this inconsistency so we can use it in existing specs that break this rule.
• When steps above are implemented, no more N+1 examples should break the inconsistency rule so we can create an epic to fix all those that use the new legacy matcher