[JBIDE-23509] made sure ProjectNameValidationTest has a conn #1670
Conversation
|
@mlabuda please review |
| } | ||
|
|
||
| closeNewProjectShell(); | ||
| new DefaultText(PROJECT_NAME_SHORT_ERROR); |
There was a problem hiding this comment.
I have same comment as in #1667 regarding test failure.
There was a problem hiding this comment.
I can only reiterate my objections:
A test with an exception is a failing test. If this is not acceptable to you then we should make sure red deer behaves differently (ex. by not throwing an exception but by throwing an AssertionFailedError that your fail() is throwing). Repeating the very same catch and fail() in tests makes no sense. It's unnecessary noise imho.
I'd like to progress here, but it looks like we are stuck in arguments. @jeffmaury @alexeykazakov further opinions please?
There was a problem hiding this comment.
I do get arguments of both sides. It's kind of frog-mice war right now. I agree with Marian that the crucial part of each test should be done by some assertion (or as he is doing it, because we don't have better mechanism right now).
But even despite that, I'm for merging this PR as it is. It hugely increases readability and it's possible (quite easily) to see from the exception where the error occurs.
We'll think of how to implement assertions into RedDeer framework so we can do this properly next time ;-)
I talked to Marian about my opinion and he agreed that this PR can be merged (but he is still not happy about that :-D).
If Jeff and Alexey want to provide opinion it's still desired. The more valid points are made, the better code we'll have.
| } | ||
|
|
||
| closeNewProjectShell(); | ||
| new DefaultText(PROJECT_NAME_FORMAT_ERROR); |
There was a problem hiding this comment.
I have same comment as in #1667 regarding test failure.
| } | ||
|
|
||
| closeNewProjectShell(); | ||
| new DefaultText(PROJECT_NAME_FORMAT_ERROR); |
There was a problem hiding this comment.
I have same comment as in #1667 regarding test failure.
| "0123456789" + | ||
| "0123456789" + | ||
| "01234"); | ||
| new DefaultText(PROJECT_NAME_MAX_LENGTH_ERROR); |
There was a problem hiding this comment.
I have same comment as in #1667 regarding test failure.
No description provided.