-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add the test for checking inactive workspace #9171
Conversation
Looks like the this PR is about |
ci-build |
+ TOASTLOADER_WIDGET_LATECY_TIMEOUT_IN_MILLISEC; | ||
ide.open(testWorkspace); | ||
projectExplorer.waitProjectExplorer(); | ||
WaitUtils.sleepQuietly(commonTimeout, TimeUnit.MILLISECONDS); |
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.
Please, explain why we need to sleep here, in comment.
package org.eclipse.che.selenium.workspaces; | ||
|
||
import static org.testng.AssertJUnit.assertEquals; | ||
|
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.
Is it exactly what you want to add? Maybe "org.testng.Assert"?
@@ -14,6 +14,8 @@ tests.screenshots_dir=target/screenshots | |||
tests.htmldumps_dir=target/htmldumps | |||
tests.workspacelogs_dir=target/workspace-logs | |||
tests.webdriverlogs_dir=target/webdriver-logs | |||
che.workspace_agent_dev_inactive_stop_timeout_ms=300000 | |||
active.state_activity_sheduler_period=60000 |
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.
Could you please rephrase the name of property so that it complies naming convention and describes the proposal clearer?
|
||
public class CheckStoppingWsByTimeoutTest { | ||
|
||
private static int TOASTLOADER_WIDGET_LATECY_TIMEOUT_IN_MILLISEC = 20000; |
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.
LATECY
- what does this mean?
|
||
@Inject | ||
@Named("che.workspace_agent_dev_inactive_stop_timeout_ms") | ||
private int stoppingTimeotInaciveWorkspace; |
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.
Timeot
- what does this mean?
int commonTimeout = | ||
stoppingTimeotInaciveWorkspace | ||
+ shedulerRequstTimeout | ||
+ TOASTLOADER_WIDGET_LATECY_TIMEOUT_IN_MILLISEC; |
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.
What about encapsulate
int commonTimeout =
stoppingTimeotInaciveWorkspace
shedulerRequstTimeout
TOASTLOADER_WIDGET_LATECY_TIMEOUT_IN_MILLISEC;
to the private method, something like "getCommonTimeout()"? This is not necessarily, but in my opinion, it will be more clean:
public void setUp() throws Exception {
ide.open(testWorkspace);
projectExplorer.waitProjectExplorer();
WaitUtils.sleepQuietly(getCommonTimeout(), TimeUnit.MILLISECONDS);
}
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.
And one more remark, typos: "stoppingTimeotInaciveWorkspace" -> "stoppingTimeoutInactiveWorkspace"
@Inject | ||
@Named("active.state_activity_sheduler_period") | ||
int shedulerRequstTimeout; | ||
|
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.
"Request"
Workspace workspace = | ||
workspaceServiceClient.getByName( | ||
testWorkspace.getName(), testUser.getName(), testUser.getAuthToken()); | ||
assertEquals(workspace.getStatus(), WorkspaceStatus.STOPPED); |
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.
Can you, please, explain why we are expecting STOPPED status of workspace here?
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/4496/ |
private int getCommonTimeout() { | ||
return cheWorkspaceAgentDevInactiveStopTimeoutMilliseconds | ||
+ TOASTLOADER_WIDGET_LATENCY_TIMEOUT_IN_MILLISEC | ||
+ cheWorkspaceActivityCheckSchedulerPeriodInSeconds * 1000; |
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.
It's better to point millisecond unit of time measurement in the method name: getCommonTimeoutInMillisec
49f5a82
to
84203a0
Compare
ci-test |
ci-test build report: |
ci-test-docker-multi |
ci-test |
ci-test build report: |
This reverts commit 8697a5e.
What does this PR do?
@artaleks9, @dmytro-ndp, @SkorikSergey, @Ohrimenko1988. @vparfonov
What issues does this PR fix or reference?
#9166