mashingan / anonimongo

Another Nim pure Mongo DB driver
MIT License
43 stars 5 forks source link

Issues in test #22

Closed samsamros closed 11 months ago

samsamros commented 1 year ago

In reviewing patches in #21, which have worked well in some of my applications, I started running the nimble tests to start tracking down issues. First issue I'm looking at. (and will follow up when I get some time during the week. ) With and without the patch, this issue arises. Nim version 1.6.14 Mongo 6.0 (haven't tried other versions) - let alone the newly released 7.0 Amd64 in Linux

[Suite] Administration APIs tests
    /home/localhost/.../anonimongo/tests/test_admmgmt_test.nim(15, 22): Check failed: mongorun.running
Error: execution of an external program failed: '/home/localhost/.../anonimongo/tests/test_admmgmt_test '
     Error: Execution failed with exit code 1
        ... Command: /home/localhost/.nimble/bin/nim c --noNimblePath -d:NimblePkgVersion=0.6.5 --path:/home/localhost/.nimble/pkgs/nimSHA2-0.1.1 --path:/home/localhost/.nimble/pkgs/scram-0.2.0 --path:/home/localhost/.nimble/pkgs/hmac-0.2.0 --path:/home/localhost/.nimble/pkgs/nimSHA2-0.1.1 --path:/home/localhost/.nimble/pkgs/sha1-1.1 --path:/home/localhost/.nimble/pkgs/nimcrypto-0.5.4 --path:/home/localhost/.nimble/pkgs/sha1-1.1 --path:/home/localhost/.nimble/pkgs/dnsclient-0.3.4 --path:/home/localhost/.nimble/pkgs/supersnappy-2.1.3 --path:/home/localhost/.nimble/pkgs/zippy-0.10.10 --path:/home/localhost/.nimble/pkgs/multisock-1.0.1 -r --path:. /home/localhost/.../anonimongo/tests/test_admmgmt_test

I have a few considerations for reviewing this and committing and pushing to development the patches in #21: 1) would be safe to apply the patch, and continue analyzing this issue separately? 2) Is opening an issue on this a good workflow to keep track of? ( I see several issues in the tests when I pass -c, maybe you're planning to do other things with them) 3) Are there things that need to be changed to make it work, I checked user, password, localhost, and other consts that don't appear to have an issue working on my setup so far. I will take a look, but opening an issue if necessary, please feel free to close if you're planning to do something else with the tests!

mashingan commented 1 year ago

Could you share your config.nims when running nimble test ? If you don't know what to fill, please look the example at tests/readme.md.

Also there's an update for the patch: types.nim.zip (just uncompress it and apply the patch)

Regarding your consideration:

  1. Yes, it's safe, the error is unrelated to the patch. The error is just config error.
  2. Yes, it's fine. Our development is in parallel so it's okay to move things on your own and merge to branch develop/master. If you still want to test before merging to develop/master, you can also push your specific branch to this repo. I do it often. If there's error or some issues, yes, opening the issue is right track.
  3. If this is related to error when running nimble test, rest assured it's not your setup issue. Very likely just config error like mentioned in answer no. 1 above.
samsamros commented 1 year ago

updating patch and testing. I only see one different line compared to the first one:

m.pickAnyServer: hostname != m.primary and theserver.pool.available.len > 0

I don't think it will have any issues with applications I'm testing with, but will let you know!

Now, I feel kind of silly for not looking closely at the consts. I was a bit tired these past days. I'm moving the tests to a VM with a clean mongodb v6 install, and will further test there. That way I can mess up the config and dbs and just go back to the snapshot where it worked :D I copied some stuff off of the git and it looks something like this:

switch("define", "filename=/home/localhost/Downloads/PROYECTO_NIM/anonimongo/tests/qrcode-me.png")
switch("define", "saveas=qrcode.png")
switch("define", "key=")
switch("define", "cert=")
switch("define", "testReplication=yes") # Planing on changing this to no, as I remember you saying there were some issues with replication?
switch("define", "testChangeStreams=yes")
switch("define", "uri")
switch("define", "existingMongoSetup")
#switch("define", "exe=D:/Installer/mongodb6.0.6/bin/mongod.exe") # didn't know what to put here in Linux so I commented it out
#switch("define", "dbpath=D:/Dev/mongodata-v6") # was scared it might affect my data on my setup, so moving to a VM to thoroughly test
switch("define", "nomongod") # I thought this might work
"ConvFromXToItselfNotNeeded".hint off
mashingan commented 1 year ago

For reference, this is my current config.nims on Linux

switch("define", "filename=")
switch("define", "saveas=")
switch("define", "user=")
switch("define", "testReplication=yes")
switch("define", "testChangeStreams=yes")
hint("ConvFromXToItSelfNotNeeded", off)
switch("define", "exe=mongod")
switch("define", "dbpath=/home/username/Apps/mongodata-v6")
switch("define", "uri")
--verbosity:0
--define:release
switch("define", "asyncBackend=chronos")

If you already have mongod running, add switch("define", "nomongod") to avoid spawning new mongod instance on port 27017. In my case, I just have mongod executable available in $PATH and I don't running mongod instance.

The dbpath=/path/of/where-to-put/the-data is argument to specifically where the persistence records will be saved. In utils_replication I pointed the dbpath to temp directory for spawning mongod in replication test.

Also for replication problem, I did solved it but haven't pushed to develop due to error in minimum Nim version. anonimongo support minimum of Nim v1.4 but one of indirect dependencies, nimcrypto, has minimum requirement of Nim v1.6. So it make the Github Action failed when testing with Nim v1.4.

samsamros commented 1 year ago

Thanks! I appreciate this I will test it in a VM and will circle back to this issue, which will most likely be closed :+1:

As for the applied patches. I have not seen an issue yet in update, aggregation, insert and findIter. Would it be safe to merge to develop, or should I further test with other applications? Seems to be working fine, I just want to test the additional line in this new patch, and will probably continue commenting about this on #21!

samsamros commented 1 year ago

I got it running in a VM... it gets all the way to replication and hangs, even though I set

switch("define", "testReplication=no")

This is where it is hanging:

[Suite] Change Stream tests
Generating a RSA private key
.................++++
..................................................++++
writing new private key to 'key.pem'
-----
  [OK] Prepare for running replica
  [OK] Setting up replica set

Is this behavior expected even when defining the test to no?

mashingan commented 1 year ago

It's actually hanging in test change streams so you had to set testChangeStream=no too. But I pushed the fix to develop so you can rebase to/merge from develop. With this fix you can safely set testReplication and testChangeStream to yes.

samsamros commented 1 year ago

thank you!! I will perform tests with fixes, and push patches over the weekend. No errors have arised so far from the patch.

samsamros commented 1 year ago

Config was definitely the problem. I'm getting the following error:

[Suite] GridFS implementation tests
  [SKIPPED] Mongo server is running
  [OK] Connect to localhost and authentication
  [OK] Drop database first in case of error in previous test
  [OK] Create default bucket
  [OK] Upload file
  [OK] Download file
fatal.nim(54)            sysFatal
asyncfutures.nim(389)    read
asyncfutures.nim(389)    read

    Unhandled exception: index 4 not in 0 .. 0 [IndexDefect]
  [FAILED] GridStream operations
    /home/testing/shared/anonimongo/tests/test_gridfs_test.nim(157, 27): Check failed: gf.getPosition == gf.fileSize - 1
    gf.getPosition was 214692
    gf.fileSize - 1 was 214691
  [FAILED] Gridstream read chunked size
  [OK] List files
  [OK] Remove file(s)
  [OK] Drop bucket
  [OK] Teardown db
  [SKIPPED] Shutdown mongo
Error: execution of an external program failed: '/home/testing/shared/anonimongo/tests/test_gridfs_test '
     Error: Execution failed with exit code 1

My config is

switch("path", "../src")
switch("define", "filename=/home/testing/Downloads/Transactions.mp")
switch("define", "saveas=code.mp")
switch("define", "user=")
switch("define", "testReplication=yes")
switch("define", "testChangeStreams=yes")
hint("ConvFromXToItSelfNotNeeded", off)
switch("define", "exe=mongod")
switch("define", "dbpath=/var/lib/mongodb")
switch("define", "uri")
switch("define", "nomongod")
--verbosity:0
--define:release
switch("define", "asyncBackend=chronos")

I'm trying this without the patch, and will run with the patch next. I rebased develop for the replication test fixes. But I think this issue is still config related. I may be wrong, though.

mashingan commented 1 year ago

Please set switch("define", "filename=") and switch("define", "saveas=") to skip testing this because the patch shouldn't related to downloading/uploading file. There's also another bug test for GridFS which I just found out it when trying on Linux but this should be separate issue I think.

samsamros commented 1 year ago

thank you! I agree I can also look into the GridFS problem and open another issue. As for this one, works perfect so far (without GridFS) with and without patches discussed in #21.

If you agree, we can close this issue and look into the GridFS stuff in another issue. I'll push the patch discussed here and #21 later today or tomorrow. Thanks so much for your time!

Btw, I tested with Mongodb 7.0 on linux amd64, tests do not work... I will open another issue at some point. I did this out of curiosity! Tests work on mongo 6.0 in amd64, no problem. and patches in #21 work in applications on a debian arm64 (rapsberry pi)

samsamros commented 11 months ago

Anonimongo v0.7.0, incorporating changes in #29; nim v2.0 linux amd64 mongodb 6 with the following config:

switch("path", "../src")
switch("define", "filename=")
switch("define", "saveas=")
switch("define", "user=")
switch("define", "testReplication=yes")
switch("define", "testChangeStreams=yes")
hint("ConvFromXToItSelfNotNeeded", off)
switch("define", "exe=mongod")
switch("define", "dbpath=/var/lib/mongodb")
switch("define", "uri")
switch("define", "nomongod")
--verbosity:0
--define:release
switch("define", "asyncBackend=chronos")

All tests pass. Except for gridfs. I will open a new issue for this and continue testing there. I will also start testing with mongo 7.0. I dont have a windows machine to test, will probably try with a virtual machine when I get the time. I think this issue can be closed, and more specific issues can be opened up depending on the config file. Do you agree @mashingan?