test: Test for Kubernetes ConfigMaps and Secrets resource pages by amisskii · Pull Request #9244 · podman-desktop/podman-desktop (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation21 Commits1 Checks13 Files changed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Signed-off-by: Anton Misskii amisskii@redhat.com
What does this PR do?
Implements tests specifically for Kubernetes ConfigMaps and Secrets resources. These tests ensure that these resources are handled correctly by the PD application.
What issues does this PR fix or reference?
How to test this PR?
- Tests are covering the bug fix or the new feature
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like all yaml files are missing the EOF/newline
| name: test-secret-resource |
|---|
| type: Opaque |
| stringData: |
| secret-username: "test-secret" |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing new line
| const PVC_POD_NAME = 'test-pod-pvcs'; |
|---|
| const CONFIGMAP_NAME = 'test-configmap-resource'; |
| const SECRET_NAME = 'test-secret-resource'; |
| const SECRETS_POD_NAME = 'test-pod-configmaps-secrets'; |
| const __filename = fileURLToPath(import.meta.url); |
| const __dirname = path.dirname(__filename); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referring to line 46 with var. skipKindInstallation Since on CI, env. vars. are of string type, here the final type of the var. is string | false. The statement can be changed to skipKindInstallation = process.env.SKIP_KIND_INSTALL === 'true' and the resulting type will be boolean.
Otherwise setting env. var. on CI to false will also lead to skipping the if on line 54.
| await playExpect(podsPage.heading).toBeVisible(); |
|---|
| const playYamlPage = await podsPage.openPlayKubeYaml(); |
| await playExpect(playYamlPage.heading).toBeVisible(); |
| const configmapYamlFilePath = path.resolve( |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we define the constants somewhere out of the test method? to make the code easily readable?
| await playExpect(podsPage.heading).toBeVisible(); |
|---|
| const playYamlPage = await podsPage.openPlayKubeYaml(); |
| await playExpect(playYamlPage.heading).toBeVisible(); |
| const secretYamlFilePath = path.resolve(__dirname, '..', '..', 'resources', 'kubernetes', `${SECRET_NAME}.yaml`); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move the constant out of test method.
| await playExpect(podsPage.heading).toBeVisible(); |
|---|
| const playYamlPage = await podsPage.openPlayKubeYaml(); |
| await playExpect(playYamlPage.heading).toBeVisible(); |
| const yamlFilePath = path.resolve(__dirname, '..', '..', 'resources', 'kubernetes', `${SECRETS_POD_NAME}.yaml`); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
| .toEqual(KubernetesResourceState.Starting); |
|---|
| }); |
| test('Bind the PVC to a pod', async ({ navigationBar }) => { |
| const podsPage = await navigationBar.openPods(); |
| await playExpect(podsPage.heading).toBeVisible(); |
| const playYamlPage = await podsPage.openPlayKubeYaml(); |
| await playExpect(playYamlPage.heading).toBeVisible(); |
| const yamlFilePath = path.resolve(__dirname, '..', '..', 'resources', 'kubernetes', `${POD_NAME}.yaml`); |
| const yamlFilePath = path.resolve(__dirname, '..', '..', 'resources', 'kubernetes', `${PVC_POD_NAME}.yaml`); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also move constant into file defition out of method. to have a little less code in the test methods.
Comment on lines 158 to 143
| await playYamlPage.playYaml(configmapYamlFilePath, { |
|---|
| runtime: PlayYamlRuntime.Kubernetes, |
| kubernetesContext: KUBERNETES_CONTEXT, |
| kubernetesNamespace: KUBERNETES_NAMESPACE, |
| }); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please extract the object into constant since it is called multiple times?
| await playExpect.poll(async () => podsDetailsPage.getState(), { timeout: 50_000 }).toEqual(PodState.Running); |
|---|
| }); |
| test('Delete the ConfigMap and Secret resources', async ({ page, navigationBar }) => { |
| const kubernetesBar = await navigationBar.openKubernetes(); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also stop and delete the pod using these resources.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Would be nice to have also a successful run on win.
This was linked to issues
Oct 18, 2024
Signed-off-by: Anton Misskii amisskii@redhat.com
Reviewers
