Cleanup SQLite tests #3246
Cleanup SQLite tests #3246
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3246 +/- ##
==========================================
- Coverage 75.20% 75.17% -0.03%
==========================================
Files 400 400
Lines 22386 22386
==========================================
- Hits 16835 16829 -6
- Misses 4574 4580 +6
Partials 977 977
... and 6 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. |
noisersup
force-pushed
the
sqlite-tests-cleanup-3131
branch
from
45601d9
to
bd4c457
Compare
noisersup
added
code/chore
| @@ -158,7 +153,7 @@ func testAggregateStagesCompatWithProviders(tt *testing.T, providers shareddata. | |||
| }) | |||
| } | |||
|
|
|||
| if tc.failsForSQLite != "" && setup.IsSQLite(tt) { | |||
| if setup.IsSQLite(tt) { | |||
There was a problem hiding this comment.
That bit is problematic: we will be looking for TODO issue URLs and "failsforsqlite" strings, and that line has neither. Makes me wonder how many problems like that we already have…
Let's add a TODO comment with an issue at least
| // TODO https://github.com/FerretDB/FerretDB/issues/3148 | ||
| if setup.IsSQLite(tt) { |
There was a problem hiding this comment.
Great, we need to do the same everywhere
| update: bson.D{{"$inc", bson.D{{"v.100", int32(42)}}}}, | ||
| }, | ||
| "DotNotationArrayFieldNotExist": { | ||
| "DotNotArrayFieldNotExist": { |
There was a problem hiding this comment.
"Not array field not exists". While the test is actually about array, not "not array".
I think that abbreviation of dot notation is unfortunate. It also does not quite belong to that PR/issue.
There was a problem hiding this comment.
We need to shorten those names as they are longer than 64 characters so they always fail
There was a problem hiding this comment.
I'm not sure why they become longer if we don't change them, but okay, let's shorten them some other non-confusing way
There was a problem hiding this comment.
They are already too long since creation. We just don't see it as they fail in the same way both on MongoDB and FerretDB. We could actually panic or fail if the name is too long to omit such situation in the future
There was a problem hiding this comment.
I replaced the names with less confusing ones, currently waiting for CI to pass, I'm also investigating why the tests were passing, although not sure if that should be covered in this PR in your opinion
There was a problem hiding this comment.
I think we should at least understand why that was happening
There was a problem hiding this comment.
Found it, it's due to the fact that we append _sqlite at the end of the database name
There was a problem hiding this comment.
If we want to avoid name mangling I guess we are forced to shorten our db names.
Description
Closes #3131.
Readiness checklist
task all, and it passed.@FerretDB/core), Milestone (Next), Labels, Project and project's Sprint fields.