jeroen / mongolite

Fast and Simple MongoDB Client for R
https://jeroen.github.io/mongolite/
286 stars 65 forks source link

Updates the specification tests to match the official new format. #146

Closed atheriel closed 6 years ago

atheriel commented 6 years ago

I tried to run the test suite locally against the specification, and noticed that it no longer works. The upstream specifications repo changed the JSON format of the test suite in c6d7ac8 -- to be precise, the keys have changed from "extjson" and "bson" to "cannonical_extjson" and "cannonical_bson", respectively, and the 128-bit tests are now in the main directory.

~This commit just moves the existing infrastructure to the new names.~

~(Note that a number of tests still fail -- I have about 1250 OKs and 30 errors locally for the specification context. I'm looking into fixing those failing tests as well.)~

The first commit just moves the existing infrastructure to the new names. Subsequent commits fix the existing failures.

jeroen commented 6 years ago

FYI I don't think the tests ever passed 100%. The mongodb folks told me this was expected because the c driver doesn't implement the entire spec.

atheriel commented 6 years ago

That's good to know. I'm interested in modifying the tests so that they account for this and track expected failures as well -- does that make sense? I'd very much like to use the spec tests to experiment with the out-of-tree changes mentioned in #37.

(Travis errors look spurious.)

atheriel commented 6 years ago

I have updated the branch to include fixes to all existing spec tests. I now have 1162 passes and zero failures locally.

jeroen commented 6 years ago

Wow that is impressive. Without editing mongolite or jsonlite?

atheriel commented 6 years ago

Ah, I realize that the binary tests only pass with my changes to jsonlite. I'll fix that.

jeroen commented 6 years ago

Could you update the specifications module to the latest version?

atheriel commented 6 years ago

Yep, done. And I've fixed the binary representation issue, along with adding an informative comment.

codecov-io commented 6 years ago

Codecov Report

Merging #146 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #146   +/-   ##
=======================================
  Coverage   36.79%   36.79%           
=======================================
  Files         129      129           
  Lines       21829    21829           
=======================================
  Hits         8032     8032           
  Misses      13797    13797

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 e7a2c13...f3e99c7. Read the comment docs.

jeroen commented 6 years ago

OK cool. I seem to have broken the Windows build in master but that is unrelated to this PR.