Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update tests #61

Merged
merged 2 commits into from Sep 21, 2016
Merged

Update tests #61

merged 2 commits into from Sep 21, 2016

Conversation

tinchodias
Copy link
Contributor

@tinchodias tinchodias commented Sep 20, 2016

Two changes in this PR:

  • Adding VORepositoryTest>>waitForWriteOperation. Reason: Write operations can require some time to be actually applied by backend, and this template method can be overriden by subclasses to do something. Do nothing by default; VOMongoTest adds a delay. This removed random failures that I have specially in travis.
  • Remove VOMongoTestResource. Reason: it's risky to share the instance of VOMongoRespository since some tests set options that can affect other tests. A repository can be created for each test run, it's not slow. I moved the code into VOMongoTest.

--> Question: Does anybody use VOMongoTestResource>>mongoHost: to customize the mongodb address? I moved it to VOMongoTest (class side, obviously).

…ions can require some time to be applied by backend, and this template method can be overriden by subclasses to do something. Do nothing by default.

Remove VOMongoTestResource: it's risky to share isntance of repository since some tests set options that can affect other tests. A repository can be created for each test run, it's not slow. I moved the code into VOMongoTest.
@tinchodias
Copy link
Contributor Author

BTW, this PR is split from #21.
In other words, if this PR is merged, #21 is almost ready for merge.

@estebanlm
Copy link
Collaborator

1 second! is not that A LOT???

@tinchodias
Copy link
Contributor Author

Ah... I thought I wrote something like 100 milliseconds. I'll change it.

@tinchodias
Copy link
Contributor Author

It's 50ms now

@estebanlm
Copy link
Collaborator

now we are talking about :)

@estebanlm estebanlm merged commit fbf00dd into pharo-nosql:master Sep 21, 2016
waitForWriteOperation
"Write operations can be performed in a fire-and-forget mode, and it's convenient to wait a bit to ensure mongodb performed it before asserting on the results."

50 milliSeconds wait
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again you can use >>#command: with getLastError and force a sync? Then the last write/delete/op must have succeeded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 this can be the next step

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants