-
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
Create a test for checking a workspace with parent functionality #22506
Conversation
4ac21a5
to
3b65333
Compare
Signed-off-by: musienko maksym <[email protected]>
3b65333
to
093a8ac
Compare
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.
a technical question
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.
Why the sudden change in formatting of package.json?
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.
Apply a prettier formatter + change some version of vscode-extension-tester
lib for correct interaction with some UI elements. But you are right it looks a bit strange. I'll take a look at the formatted part again and change it if it is incorrect
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.
thank you for explanation and verification
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.
nice, thank you for this implementation
import { Locators, ModalDialog } from 'monaco-page-objects'; | ||
import { CheCodeLocatorLoader } from '../pageobjects/ide/CheCodeLocatorLoader'; | ||
|
||
const webCheCodeLocators: Locators = new CheCodeLocatorLoader().webCheCodeLocators; |
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.
class should be injected
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.
); | ||
expect(envList).contain('DEVFILE_ENV_VAR=true'); | ||
expect(envList).contain('PARENT_ENV_VAR=true'); | ||
}); |
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.
should be logout to run few test specs at once
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.
implemented: #22544
const dashboard: Dashboard = e2eContainer.get(CLASSES.Dashboard); | ||
const shellExecutor: ShellExecutor = e2eContainer.get(CLASSES.ShellExecutor); | ||
const workspaceHandlingTests: WorkspaceHandlingTests = e2eContainer.get(CLASSES.WorkspaceHandlingTests); | ||
function executeArbitraryShellScript(command: string): string { |
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.
better move to ShellExecutor class
import { ShellString } from 'shelljs'; | ||
import { WorkspaceHandlingTests } from '../../tests-library/WorkspaceHandlingTests'; | ||
import { registerRunningWorkspace } from '../MochaHooks'; | ||
const projectAndFileTests: ProjectAndFileTests = e2eContainer.get(CLASSES.ProjectAndFileTests); |
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.
constants should be declarated inside suit
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.
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.
implemented: #22544
registerRunningWorkspace(WorkspaceHandlingTests.getWorkspaceName()); | ||
await projectAndFileTests.waitWorkspaceReadinessForCheCodeEditor(); | ||
// sometimes the trust dialog does not appear at first time, for avoiding this problem we send click event for activating | ||
await new Workbench().click(); |
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 it be inside projectAndFileTests.performTrustAuthorDialog() method?
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.
implemented: #22544
await devFileTask?.click(); | ||
const firstExpectedQuickPick: QuickPickItem | undefined = await input.findQuickPick('1. This command from the devfile'); | ||
const secondExpectedQuickPick: QuickPickItem | undefined = await input.findQuickPick('2. This command from the parent'); | ||
expect(firstExpectedQuickPick).not.eqls(undefined); |
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 be just .not.undefined
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.
implemented: #22544
|
||
test('Check expected environment variables', function (): void { | ||
const envList: string = executeArbitraryShellScript( | ||
`${API_TEST_CONSTANTS.TS_API_TEST_KUBERNETES_COMMAND_LINE_TOOL} exec -i ${podName} -c tools -- sh -c env` |
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.
Shell we add it into KubernetesCommandLineToolsExecutor? Like getContainerEnvs()
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.
Usually, we do not use it in other tests. It is specific for this functionality. But if it repeats in other test parts - of course, it is better to extract it. My point is to avoid methods that will be used for one test. In this case, we do not overload KubernetesCommandLineToolsExecutor
class.
expect(containerNames).contains('che-gateway'); | ||
|
||
const initContainerName: string = executeArbitraryShellScript( | ||
`${API_TEST_CONSTANTS.TS_API_TEST_KUBERNETES_COMMAND_LINE_TOOL} get pod ${podName} --output jsonpath=\'{.spec.initContainers[].name}\'` |
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 we use is it getPodAndContainerNames() here also? mb with some updates?
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.
So it makes sense to extract it if we use it regularly. But WorkpaseUsingParent.spec.ts test is pretty specific. These specific commands are used in this test. But if we get precedent to use the same command in future tests, it will be better to extract in a method.
}); | ||
|
||
test('Check expected containers in the parent POD', function (): void { | ||
const getPodNameCommand: string = `${API_TEST_CONSTANTS.TS_API_TEST_KUBERNETES_COMMAND_LINE_TOOL} get pods --selector=controller.devfile.io/devworkspace_name=sample-using-parent --output jsonpath=\'{.items[0].metadata.name}\'`; |
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`t check test suite locally because loginToOcp() method is missing.
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.
Well, I supposed if we run it on CI - it will be applied approach like in the PredefinedNamespaceTest: https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/job/Testing/job/e2e/job/basic/job/typescript-tests/12044/console, and mount kubeconfig directory: -v /home/hudson/.kube:/home/seluser/.kube:Z
this file stores sessions with logins. And we do not need to do it in the test twice. If you want to check it locally, you can log in with oc client manually and run the test as well.
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 is additional manual steps to remember. method checks firstly if user already logged.
https://github.com/eclipse/che/blob/17b7a3e8e0e3138ddfadd95414a97e931e4667d0/tests/e2e/specs/miscellaneous/PredefinedNamespace.spec.ts#L38C14-L38C14
in the PredefinedNamespaceTest we have login method after refactoring
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.
Applied in the #22544
What does this PR do?
Automate the scenario described in the issue: https://issues.redhat.com/browse/CRW-4653
Screenshot/screencast of this PR
https://gitlab.cee.redhat.com/codeready-workspaces/test-cases/-/blob/master/tests/functional/git/m52-test-workspace-with-devfile-which-uses-the-parent.md
What issues does this PR fix or reference?
https://issues.redhat.com/browse/CRW-4653
How to test this PR?
Set up a test infra with the DevSpaces v.3.8 or later. Set up USERSTORY=WorkpaseUsingParent and other dedicated envs for the test and run
An example with successful test:
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or reference
andHow to test this PR
completedReviewers
Reviewers, please comment how you tested the PR when approving it.