Skip to content
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

Fix Helm repositories and pod logs integration tests #2015

Merged
merged 2 commits into from Jan 26, 2021

Conversation

nevalla
Copy link
Contributor

@nevalla nevalla commented Jan 25, 2021

Signed-off-by: Lauri Nevala lauri.nevala@gmail.com

Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>
Comment on lines 46 to 60
export async function listHelmRepositories(retries = 0): Promise<string> {
if (retries < 5) {
try {
const { stdout: reposJson } = await promiseExec("helm repo list -o json");

return reposJson;
} catch {
await new Promise(r => setTimeout(r, 2000)); // if no repositories, wait for Lens adding bitnami repository

return await listHelmRepositories((retries + 1));
}
}

return "[]";
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really think that this should return any[] or maybe even { name: string, url: string}[]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @Nokel81 , feels weird to return string and not array of objects (basically parsed json).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it's not parsed json that is returned. Should we return parsed json instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it's not parsed json that is returned. Should we return parsed json instead?

I think it would make more sense to return parsed json.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

integration/__tests__/app.tests.ts Show resolved Hide resolved
@jakolehm jakolehm added area/ci bug Something isn't working blocker labels Jan 26, 2021
@jakolehm jakolehm added this to the 4.1.0 milestone Jan 26, 2021
Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>
let podMenuItemEnabled = false;

// Wait until extensions are enabled on renderer
while (!podMenuItemEnabled) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for this PR but I think we need to have a better fix for this in future. Buttons should appear when an extension gets loaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

await app.client.waitForVisible(".LogResourceSelector");
await app.client.waitForVisible(".LogResourceSelector .SearchInput");
await app.client.waitForVisible(".LogResourceSelector .SearchInput input");
//await app.client.waitForVisible(".LogSearch .SearchInput");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A line to remove probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes...forgot to remove that line after testing that it's redundant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove it next time (trying not to dismiss an approvements).

@jakolehm jakolehm merged commit 5f19606 into master Jan 26, 2021
@jakolehm jakolehm deleted the fix-helm-repo-pod-logs-integration-tests branch January 26, 2021 11:26
@jakolehm jakolehm mentioned this pull request Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci blocker bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants