test: fix unit tests #31
Conversation
test: fix order problem test: try to fix transaction test: try to fix ci test: try to fix ci test: try to fix ci test: try to fix ci test: try to fix ci
| @@ -230,6 +230,7 @@ export class MongooseAdapter { | |||
| if (this.isSynced) { | |||
| const session = await this.getSession(); | |||
| if (!session.inTransaction()) { | |||
| await CasbinRule.createCollection() | |||
There was a problem hiding this comment.
Hmm. Is this really necessary? We use this library in production and also test environments where we rebuild database constantly and we have never seen this to be an issue.
There was a problem hiding this comment.
Mongodb will automatic execute this, but I think explicitly defined is also ok.
There was a problem hiding this comment.
Hmm. I would probably then move it to connect-function if adding it is necessary.
I have not seen this as common practice, so it would be good to add a comment to this as well on why createCollection is added. However, I might be wrong.
Now it was built every time. |
| CONFIG="{ _id: '$REPLICA_SET_NAME', members: [{_id: 0, host: 'localhost:27001', priority: 2 }, { _id: 1, host: 'localhost:27002' }, { _id: 2, host: 'localhost:27003' } ]}" | ||
| mongo admin --port 27001 --eval "db.runCommand({ replSetInitiate: $CONFIG })" | ||
|
|
||
| waitForMongo 27002 |
There was a problem hiding this comment.
While this might be sufficient I think it's operationally different. rs_status previously tried to verify that replicaset is valid.
The difference here might be that db-command could go through even when replicaset configuration is not yet done. (For example voting for primary / secondary / arbiter might not be done yet, but database accepts requests)
While waitForMongo can be sufficient to get the database responsive, I am not sure whether it means that the replicaset is fully operational.
This might be just splitting hairs , but I would try to avoid any unexpected errors.
There was a problem hiding this comment.
I think this won't be a problem. Since after this CI will execute yarn install then.
Mongo has enought time to finish its work.
There was a problem hiding this comment.
Yeah, you are probably right.
Fix: #29
I need to explain what I do to solve this problem.
If you choose to use transaction with mongodb, even it have returned a success code, it still needs some time to setup the collections. If you operate it at this time, you will got something like
I didn't find some better way to solve this, and seems it only happens when you run unnittests. It's almost impossible to reproduce this in production environment. So I add
ok, now it works fine.
If you confused about this meaningless code line. Plz read this.
Don't remove it.