Add missing "is" assertions to various tests

[jeff303]

Many tests were missing the actual assertion, and only had an = invocation.

[tsmacdonald]

Good catch! LGTM if green

[codecov]

Codecov Report

Merging #15254 (0911de0) into master (3099c35) will increase coverage by 0.00%.
The diff coverage is 88.61%.

@@           Coverage Diff           @@
##           master   #15254   +/-   ##
=======================================
  Coverage   84.42%   84.42%           
=======================================
  Files         390      394    +4     
  Lines       30903    31001   +98     
  Branches     2213     2229   +16     
=======================================
+ Hits        26089    26173   +84     
+ Misses       2601     2599    -2     
- Partials     2213     2229   +16     

Impacted Files	Coverage Δ
src/metabase/models.clj	100.00% <ø> (ø)
src/metabase/server/handler.clj	66.66% <ø> (ø)
src/metabase/server/middleware/util.clj	100.00% <ø> (+21.42%)	⬆️
...tabase/query_processor/middleware/large_int_id.clj	87.50% <50.00%> (-8.16%)	⬇️
src/metabase/server/middleware/session.clj	91.86% <50.00%> (+0.07%)	⬆️
src/metabase/util/files.clj	51.61% <50.00%> (+2.40%)	⬆️
src/metabase/models/login_history.clj	78.57% <78.57%> (ø)
.../src/metabase_enterprise/sso/integrations/saml.clj	72.89% <80.00%> (+2.60%)	⬆️
src/metabase/server/middleware/browser_cookie.clj	82.35% <82.35%> (ø)
src/metabase/server/request/util.clj	86.36% <86.36%> (ø)
... and 26 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba495bd...0911de0. Read the comment docs.

[jeff303]

It seems that row-type-agnostic-test is an interesting failure, here. The failing assertion uses strings for the expected values, but the behavior is inconsistent. Some drivers are actually returning numeric values instead, causing the test to fail, like

:oracle All QP middleware should work regardless of the type of each row (#13475) rows = ^clojure.lang.PersistentVector [[1] [2147483647]] Should work with the middleware options used by API requests as well
expected: [["1"] ["2147483647"]]
  actual: [[1] [2147483647]]
    diff: - [["1"] ["2147483647"]]
          + [[1] [2147483647]]

Oracle and Snowflake are failing in this way. But switching the expected values to numeric instead makes them pass, but obviously then the others fail (ex: H2, Postgres, etc.)

I can obviously fix the assertion by converting these values to numerics before asserting, but would that possibly be masking some type of driver or middleware level issue? Any thoughts, @camsaul ?

[camsaul]

metabase.query-processor.reducible-test/row-type-agnostic-test is failing for Oracle and Snowflake now, doesn't look like a flake.

[jeff303]

metabase.query-processor.reducible-test/row-type-agnostic-test is failing for Oracle and Snowflake now, doesn't look like a flake.

It's not. Please refer to my other comment just before.

[jeff303]

Tracked down the problem, which was in the convert-id-to-string middleware. Apparently, Oracle and Snowflake sync the primary key columns for our sample datasets (and, presumably, all datasets), as :type/Number instead of :type/Integer or :type/BigInteger, which was the only thing that middleware fn was considering before.

[camsaul]

LGTM