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

ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FILE_OPEN_TIMEOUT value #2460

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@feuillemorte
Copy link
Contributor

feuillemorte commented Dec 11, 2018

Added 2 tests to cover changing variable DEFAULT_JOURNAL_FILE_OPEN_TIMEOUT:

  1. I'm starting a server and then checking that I have default value
  2. I'm setting my random value, then starting a server and then checking that server has my value not default
@michalxo

This comment has been minimized.

Copy link

michalxo commented Dec 12, 2018

@mnovak1 @andytaylor could you please check this PR?

@mnovak1

This comment has been minimized.

Copy link
Contributor

mnovak1 commented Dec 12, 2018

@michalxo Would it be possible to check that value was set to JournalFilesRepository.journalFileOpenTimeout attribute?

@michalxo

This comment has been minimized.

Copy link

michalxo commented Dec 13, 2018

It is a question on @feuillemorte Oleg, not me. I am not an author :-)

@franz1981

This comment has been minimized.

Copy link
Contributor

franz1981 commented Dec 13, 2018

@mnovak1 @feuillemorte JournalFilesRepository.journalFileOpenTimeout isn't public or exposed by any getter hence to check it can be done in 2 ways:

  1. using a mock to intercept it when created
  2. exposing it with a getter on JournalFilesRepository so it can be verified

AFAIK we are not doing it for the other classes, so I think it can be avoided.
But if we have some good reason to do it, @feuillemorte can find a proper way to do it 👍

@mnovak1

This comment has been minimized.

Copy link
Contributor

mnovak1 commented Dec 13, 2018

@michalxo Sorry, i got confused as you were asking to check this PR :-)

@feuillemorte @franz1981 Also byteman might do the trick. Byteman rule would be triggered after write to journalFileOpenTimeout variable. Rule would call byteman helper method in test case which would set some variable to given value. Then this value would be asserted in test. I think that there are already some tests like this in Arteims test suite.

@feuillemorte feuillemorte force-pushed the feuillemorte:ARTEMIS-2201_default_journal_file_open_timeout branch from b9bd5cf to fcc8e76 Dec 13, 2018

@feuillemorte feuillemorte force-pushed the feuillemorte:ARTEMIS-2201_default_journal_file_open_timeout branch from fcc8e76 to cec7218 Dec 13, 2018

@feuillemorte

This comment has been minimized.

Copy link
Contributor Author

feuillemorte commented Dec 13, 2018

seems that there is no byteman in mvn dependencies in this package. I don't know is it a good idea to add it to dependencies... So, I would like to add getter for journalFileOpenTimeout variable. @franz1981

@mnovak1

This comment has been minimized.

Copy link
Contributor

mnovak1 commented Dec 13, 2018

Test looks ok to me now.

@asfgit asfgit closed this in e41a24a Dec 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.