catch the TimeoutException #9
Conversation
src/main/java/org/jivesoftware/smackx/pubsub/PubSubIntegrationTest.java
Outdated
Show resolved
Hide resolved
| * <p> | ||
| * From XEP-0060 § 8.4.1: | ||
| * </p> | ||
| * <blockquote> In order to delete a node, a node owner MUST send a node |
There was a problem hiding this comment.
This isn't the correct quote. It does not describe what you're testing here!
There was a problem hiding this comment.
This is not yet the complete test. It's just a step
There was a problem hiding this comment.
I know. Just leave this comment open until you fixed it. :)
There was a problem hiding this comment.
The quote still is not correct.
There was a problem hiding this comment.
I have already changed the quote
src/main/java/org/jivesoftware/smackx/pubsub/PubSubIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jivesoftware/smackx/pubsub/PubSubIntegrationTest.java
Outdated
Show resolved
Hide resolved
| * <p> | ||
| * From XEP-0060 § 8.4.1: | ||
| * </p> | ||
| * <blockquote> In order to delete a node, a node owner MUST send a node |
There was a problem hiding this comment.
The quote still is not correct.
| NotConnectedException, InterruptedException, TimeoutException, ExecutionException { | ||
| final String nodename = "sinttest-delete-node-that-exist-" + testRunId; | ||
| final String needle = "<event xmlns='http://jabber.org/protocol/pubsub#event'>"; | ||
| final String delete_confirm = "<delete node='princely_musings'>"; |
There was a problem hiding this comment.
This won't work. You're creating a node wit a name that starts with sinttest-delete-node-that-exists-, but you're deleting a node that is named princely_musings.
| assertNotNull(result.get(conOne.getReplyTimeout(), TimeUnit.MILLISECONDS)); | ||
| } | ||
| catch (PubSubException.NotAPubSubNodeException e) { | ||
| throw new AssertionError("The published item was not received by the subscriber.", e); |
There was a problem hiding this comment.
A test can have three results:
- All assertions pass: test succeeds. This means that the test was executed, and the thing that we are testing is working as expected.
- One of the assertions does not pass: test fails. This means that the test was executed, and the thing that we are testing is not working as expected.
- An error occurs: tests in error. This means that the test was not executed properly at all. This might indicate that there is a problem with the test code, or something else unexpected happened.
Assertions are used to verify expected behavior. When you write a test, you test for a very specific thing to happen. That is where you use an assertion: if the things does not happen, the assertion fails. This means that the test fails.
If something unexpected happens - something that went wrong, without it being the subject of the test - then the test should exit in error.
By catching PubSubException.NotAPubSubNodeException and throwing an AssertionError you are causing the test to fail. Is that really what you want here (are you testing a scenario in which a PubSubException.NotAPubSubNodeException can be expected to happen, normally?) I think that instead, the code should simply cause the original exception to be thrown. You don't need to 'catch' it.
There was a problem hiding this comment.
sometimes I used to have errors due to the duration too long when I made tests
… element s, catch a TimeoutException
|
sometimes I used to have errors due to the duration too long when I made tests |
yannickchoni
changed the title
No description provided.