You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by feuillemorte <gi...@git.apache.org> on 2018/12/11 14:11:49 UTC
[GitHub] activemq-artemis pull request #2460: ARTEMIS-2201 Added tests on DEFAULT_JOU...
GitHub user feuillemorte opened a pull request:
https://github.com/apache/activemq-artemis/pull/2460
ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FILE_OPEN_TIMEOUT value
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/feuillemorte/activemq-artemis ARTEMIS-2201_default_journal_file_open_timeout
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2460.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2460
----
commit b67a624a5282e263938351ac89c21b01734609cf
Author: feuillemorte <it...@...>
Date: 2018-12-11T14:05:45Z
#ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FILE_OPEN_TIMEOUT value
----
---
[GitHub] activemq-artemis issue #2460: ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FI...
Posted by mnovak1 <gi...@git.apache.org>.
Github user mnovak1 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2460
@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.
---
[GitHub] activemq-artemis pull request #2460: ARTEMIS-2201 Added tests on DEFAULT_JOU...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2460
---
[GitHub] activemq-artemis issue #2460: ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FI...
Posted by mnovak1 <gi...@git.apache.org>.
Github user mnovak1 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2460
@michalxo Would it be possible to check that value was set to JournalFilesRepository.journalFileOpenTimeout attribute?
---
[GitHub] activemq-artemis issue #2460: ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FI...
Posted by michalxo <gi...@git.apache.org>.
Github user michalxo commented on the issue:
https://github.com/apache/activemq-artemis/pull/2460
It is a question on @feuillemorte Oleg, not me. I am not an author :-)
---
[GitHub] activemq-artemis issue #2460: ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FI...
Posted by feuillemorte <gi...@git.apache.org>.
Github user feuillemorte commented on the issue:
https://github.com/apache/activemq-artemis/pull/2460
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
---
[GitHub] activemq-artemis issue #2460: ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FI...
Posted by mnovak1 <gi...@git.apache.org>.
Github user mnovak1 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2460
Test looks ok to me now.
---
[GitHub] activemq-artemis issue #2460: ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FI...
Posted by michalxo <gi...@git.apache.org>.
Github user michalxo commented on the issue:
https://github.com/apache/activemq-artemis/pull/2460
@mnovak1 @andytaylor could you please check this PR?
---
[GitHub] activemq-artemis issue #2460: ARTEMIS-2201 Added tests on DEFAULT_JOURNAL_FI...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2460
@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 :+1:
---