-
Notifications
You must be signed in to change notification settings - Fork 50
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
[JBIDE-23509] made sure ProjectNameValidationTest has a conn #1670
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/******************************************************************************* | ||
* Copyright (c) 2007-2016 Red Hat, Inc. | ||
* Copyright (c) 2016 Red Hat, Inc. | ||
* Distributed under license by Red Hat, Inc. All rights reserved. | ||
* This program is made available under the terms of the | ||
* Eclipse Public License v 1.0 which accompanies this distribution, | ||
|
@@ -10,9 +10,6 @@ | |
******************************************************************************/ | ||
package org.jboss.tools.openshift.ui.bot.test.project; | ||
|
||
import static org.junit.Assert.fail; | ||
|
||
import org.jboss.reddeer.common.exception.RedDeerException; | ||
import org.jboss.reddeer.common.wait.TimePeriod; | ||
import org.jboss.reddeer.common.wait.WaitWhile; | ||
import org.jboss.reddeer.core.condition.ShellWithTextIsAvailable; | ||
|
@@ -21,11 +18,19 @@ | |
import org.jboss.reddeer.swt.impl.shell.DefaultShell; | ||
import org.jboss.reddeer.swt.impl.text.DefaultText; | ||
import org.jboss.reddeer.swt.impl.text.LabeledText; | ||
import org.jboss.tools.openshift.reddeer.requirement.OpenShiftConnectionRequirement.RequiredBasicConnection; | ||
import org.jboss.tools.openshift.reddeer.utils.OpenShiftLabel; | ||
import org.jboss.tools.openshift.reddeer.view.OpenShiftExplorerView; | ||
import org.jboss.tools.openshift.reddeer.view.resources.OpenShift3Connection; | ||
import org.junit.After; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
|
||
/** | ||
* @author mlabuda@redhat.com | ||
* @author adietish@redhat.com | ||
*/ | ||
@RequiredBasicConnection | ||
public class ProjectNameValidationTest { | ||
|
||
public static final String PROJECT_NAME_FORMAT_ERROR = | ||
|
@@ -38,61 +43,51 @@ public class ProjectNameValidationTest { | |
public static final String PROJECT_NAME_MAX_LENGTH_ERROR = | ||
" Maximum length allowed is 63 characters for project name"; | ||
|
||
@Before | ||
public void setUp() { | ||
openNewProjectShell(); | ||
} | ||
|
||
@After | ||
public void cleanUp() { | ||
closeNewProjectShell(); | ||
} | ||
|
||
@Test | ||
public void testShortProjectName() { | ||
openNewProjectShell(); | ||
new LabeledText(OpenShiftLabel.TextLabels.PROJECT_NAME).setText("s"); | ||
|
||
try { | ||
new DefaultText(PROJECT_NAME_SHORT_ERROR); | ||
} catch (RedDeerException ex) { | ||
fail("There is no validation message warning about small length of a project name."); | ||
} | ||
|
||
closeNewProjectShell(); | ||
new DefaultText(PROJECT_NAME_SHORT_ERROR); | ||
} | ||
|
||
@Test | ||
public void testInvalidProjectNameFormat() { | ||
openNewProjectShell(); | ||
new LabeledText(OpenShiftLabel.TextLabels.PROJECT_NAME).setText("--"); | ||
|
||
try { | ||
new DefaultText(PROJECT_NAME_FORMAT_ERROR); | ||
} catch (RedDeerException ex) { | ||
fail("There is no validation message warning about inappropriate project name."); | ||
} | ||
|
||
closeNewProjectShell(); | ||
new DefaultText(PROJECT_NAME_FORMAT_ERROR); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have same comment as in #1667 regarding test failure. |
||
} | ||
|
||
@Test | ||
public void testForbiddenCharactersInProjectName() { | ||
openNewProjectShell(); | ||
new LabeledText(OpenShiftLabel.TextLabels.PROJECT_NAME).setText("AAA"); | ||
|
||
try { | ||
new DefaultText(PROJECT_NAME_FORMAT_ERROR); | ||
} catch (RedDeerException ex) { | ||
fail("There is no validation message warning about inappropriate project name."); | ||
} | ||
|
||
closeNewProjectShell(); | ||
new DefaultText(PROJECT_NAME_FORMAT_ERROR); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have same comment as in #1667 regarding test failure. |
||
} | ||
|
||
@Test | ||
public void testLongProjectName() { | ||
openNewProjectShell(); | ||
new LabeledText(OpenShiftLabel.TextLabels.PROJECT_NAME).setText("012345678901234567890" + | ||
"12345678901234567890123456789012345678901234567890123456789012345678901234567890" + | ||
"123456789012345678901234"); | ||
try { | ||
new DefaultText(PROJECT_NAME_MAX_LENGTH_ERROR); | ||
} catch (RedDeerException ex) { | ||
fail("There is no validation message warning about small length of a project name."); | ||
} | ||
|
||
closeNewProjectShell(); | ||
new LabeledText(OpenShiftLabel.TextLabels.PROJECT_NAME).setText( | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"0123456789" + | ||
"01234"); | ||
new DefaultText(PROJECT_NAME_MAX_LENGTH_ERROR); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have same comment as in #1667 regarding test failure. |
||
} | ||
|
||
private void openNewProjectShell() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have same comment as in #1667 regarding test failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.