Skip to content

Fix some type issues in x-pack/test_serverless#167346

Merged
delanni merged 6 commits intoelastic:mainfrom
Ikuni17:fix/x-pack-test-serverless-types
Sep 27, 2023
Merged

Fix some type issues in x-pack/test_serverless#167346
delanni merged 6 commits intoelastic:mainfrom
Ikuni17:fix/x-pack-test-serverless-types

Conversation

@Ikuni17
Copy link
Contributor

@Ikuni17 Ikuni17 commented Sep 26, 2023

Summary

We're breaking #166813 up into smaller PRs in the interest of getting PRs through sooner for type fixes. These are the changes for x-pack/test_serverless.

Reviewers

There are no code owners for these files, so I'm using the recently edited suggestions

@Ikuni17 Ikuni17 added Team:Operations Kibana-Operations Team release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v8.11.0 labels Sep 26, 2023
@Ikuni17 Ikuni17 self-assigned this Sep 26, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@Ikuni17 Ikuni17 requested review from a team and paul-tavares September 26, 2023 20:39
Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

LGTM 👍

*/
export const login: CyLoginTask = (
user: ServerlessRoleName | 'elastic' = 'soc_manager'
user: ServerlessRoleName | 'elastic' = ServerlessRoleName.SOC_MANAGER
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you 🙏

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

LGTM

@Ikuni17 Ikuni17 enabled auto-merge (squash) September 26, 2023 21:26
delanni and others added 2 commits September 27, 2023 09:52
optional chaining is safer in these cases, and the accepting function doesn't care about the resulting types

Co-authored-by: Thomas Watson <w@tson.dk>
Comment on lines +17 to +22
export interface User {
username: string;
password: string;
description?: string;
roles: 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 can't see that User is used outside of this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still verifying these, but I these kinds of "export" additions were a kind of typescript errors that were something like an inferred type was private - these additions fixed those errors, but now that I'm re-verifying, both User and CommonSavedObjectAttributes works without export, so I'm puzzled 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

IDE TypeScript errors work in mysterious ways 🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

it wasn't an IDE, it was through the typecheck command. But it no longer seems to be present, so we can undo these, or keep them

Comment on lines +12 to +18
export interface CommonSavedObjectAttributes {
id?: string | null;
created_at?: string | null;
updated_at?: string | null;
version?: string | null;
[key: string]: unknown;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see that CommonSavedObjectAttributes is used outside of this file?

@delanni
Copy link
Contributor

delanni commented Sep 27, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link

kibana-ci commented Sep 27, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #11 / serverless examples UI Partial Results Example "before all" hook for "should trace mouse events"

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @delanni @Ikuni17

@delanni delanni disabled auto-merge September 27, 2023 13:22
@delanni delanni merged commit 563292f into elastic:main Sep 27, 2023
@Ikuni17 Ikuni17 deleted the fix/x-pack-test-serverless-types branch September 28, 2023 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Operations Kibana-Operations Team v8.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants