Conversation
autarch
force-pushed
the
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
branch
from
6a8bc27 to
ce94d63
Compare
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
97fa2e2 to
a95e9d2
Compare
autarch
changed the base branch from
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
to
graphite-base/925
autarch
force-pushed
the
graphite-base/925
branch
from
ce94d63 to
93cafd5
Compare
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
a95e9d2 to
10d6e5e
Compare
autarch
changed the base branch from
graphite-base/925
to
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
autarch
force-pushed
the
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
branch
from
93cafd5 to
d6504ac
Compare
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
10d6e5e to
1e1b554
Compare
autarch
force-pushed
the
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
branch
from
a0ef32f to
b457216
Compare
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
65fd1be to
0714696
Compare
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
0714696 to
cef05b8
Compare
autarch
force-pushed
the
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
branch
from
b457216 to
9db3a63
Compare
autarch
changed the base branch from
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
to
graphite-base/925
mmcclimon
left a comment
mmcclimon
left a comment
There was a problem hiding this comment.
I also left this comment like 5 minutes ago on the DSC testing thing, but: I think it would be useful to have a conversation about where to put / how to do these round-trip tests. I recently added (at your prompting) an integration/importexport directory to put these kinds of tests, because I feel like it's sort of weird to put them in either mongoimport/ or mongoexport/.
Like this test is fine, and we can always move them later if we want, but it would also be nice if we didn't have to and could just rewrite them all once. The tests I wrote also used the testify suite, which I prefer to passing t around everywhere, and I'd imagined that the other e2e-style tests would do similar, and we probably don't want to rewrite them all twice (once to Go, once to suite-style). We don't have to use the suites, of course, but we do in mongosync and I would love to have one way to write tests instead of 7.
mongoimport/mongoimport_test.go
Outdated
| require.NoError(t, err) | ||
| assert.EqualValues(t, 50, count, "collection should have all 50 documents after round-trip") | ||
|
|
||
| for i := range int32(50) { |
There was a problem hiding this comment.
Your robot is obsessed with int32s, man.
There was a problem hiding this comment.
I think that's because the existing test code is constantly doing this already, and it's trying to match the existing style. Why does the existing code do this? I dunno 🤷
There was a problem hiding this comment.
I was wondering if it was like, "well, numbers in JS are all floats, so we might not always be able to fit a 64-bit int" or something.
|
I actually was thinking of moving them after doing a bunch of these test conversions. At this point, I'd like to just merge these as-is and do some cleanup after. I also agree that moving to using a testify suite would be good. But I'd also like to defer that until after mergint his mega-stack. |
autarch
force-pushed
the
graphite-base/925
branch
from
9db3a63 to
41037a4
Compare
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
cef05b8 to
37051b2
Compare
autarch
changed the base branch from
graphite-base/925
to
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
mmcclimon
left a comment
mmcclimon
left a comment
There was a problem hiding this comment.
I actually was thinking of moving them after doing a bunch of these test conversions. At this point, I'd like to just merge these as-is and do some cleanup after. I also agree that moving to using a testify suite would be good. But I'd also like to defer that until after mergint his mega-stack.
Cool, sounds good to me; I just wanted to make sure we were vaguely aligned on the direction we want to shove on these.
Change LGTM, thanks!
|
I went through the entire stack and fixed all the many places it pointlessly used an |
autarch
changed the base branch from
03-19-rewrite_all_the_bsondump_tests_in_go_and_delete_the_js_tests_for_this
to
graphite-base/925
autarch
force-pushed
the
graphite-base/925
branch
from
41037a4 to
bbb8792
Compare
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
37051b2 to
ab92ff8
Compare
This PR adds a new `TestRoundTripBasicData` test to `mongoimport/mongoimport_test.go`. The test inserts 50 docs, exports them as JSON, and then imports them to test a basic round trip scenario.
autarch
force-pushed
the
03-20-convert_test/qa-tests/jstests/export/basic_data.js_to_go
branch
from
ab92ff8 to
6805d1c
Compare
2 participants
This PR adds a new
TestRoundTripBasicDatatest tomongoimport/mongoimport_test.go. The test inserts 50 docs, exports them as JSON, and then imports them to test a basic round trip scenario.