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.

Sign up for GitHub

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

Jump to bottom

Fix some type issues in x-pack/test #167343

Merged
merged 12 commits into from
Sep 27, 2023
Merged

Fix some type issues in x-pack/test #167343

merged 12 commits into from
Sep 27, 2023

Conversation

Ikuni17
Copy link
Contributor

@Ikuni17 Ikuni17 commented Sep 26, 2023 •
edited
Loading

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.

@Ikuni17 Ikuni17 added Team:Operations Team label for Operations Team release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.11.0 labels Sep 26, 2023
@Ikuni17 Ikuni17 self-assigned this Sep 26, 2023
@Ikuni17 Ikuni17 requested review from a team as code owners September 26, 2023 20:18
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot added Team:APM All issues that need APM UI Team support Team:Fleet Team label for Observability Data Collection Fleet team labels Sep 26, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@Ikuni17 Ikuni17 changed the title Fix type issues in x-pack/test Fix some type issues in x-pack/test Sep 26, 2023
@Ikuni17 Ikuni17 enabled auto-merge (squash) September 26, 2023 21:26
sorenlouv
sorenlouv approved these changes Sep 27, 2023
View reviewed changes
Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

apm changes lgtm

watson
watson reviewed Sep 27, 2023
View reviewed changes
x-pack/test/api_integration/apis/management/ingest_pipelines/lib/fixtures.ts
@@ -12,7 +12,7 @@ import {
IngestPutPipelineRequest,
} from '@elastic/elasticsearch/lib/api/types';

interface Pipeline {
export interface Pipeline {
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 this 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.

maybe it's no longer relevant, but it was used in another time when this was fixed :/

x-pack/test/api_integration/apis/management/ingest_pipelines/lib/fixtures.ts Show resolved Hide resolved
delanni
delanni reviewed Sep 27, 2023
View reviewed changes
...engine_api_integration/security_and_spaces/group9/exception_operators_data_types/ip_array.ts
@@ -152,7 +152,7 @@ export default ({ getService }: FtrProviderContext) => {
await waitForSignalsToBePresent(supertest, log, 1, [id]);
const signalsOpen = await getSignalsById(supertest, log, id);
const ips = signalsOpen.hits.hits.map((hit) => hit._source?.ip).sort();
expect(ips.flat(Number.MAX_SAFE_INTEGER)).to.eql([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

this would kill the typescript typecheck by saying the expanded type is infinitely deep

@delanni
Copy link
Contributor

delanni commented Sep 27, 2023

@elasticmachine merge upstream

@delanni
Copy link
Contributor

delanni commented Sep 27, 2023

@elasticmachine merge upstream

ymao1
ymao1 approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

response ops changes lgtm

@delanni delanni requested review from a team as code owners September 27, 2023 14:00
@delanni delanni force-pushed the fix/x-pack-test-types branch from d4e5c0a to 7b58719 Compare September 27, 2023 16:21
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #10 / serverless security UI Case View page "before all" hook for "should show the case view page correctly"

Metrics [docs]

✅ unchanged

History

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

cc @delanni @Ikuni17

delanni
delanni approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@delanni delanni left a comment

Choose a reason for hiding this comment

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

Looks good, let's merge

@Ikuni17 Ikuni17 disabled auto-merge September 27, 2023 20:13
@Ikuni17 Ikuni17 merged commit f9c2620 into elastic:main Sep 27, 2023
@Ikuni17
Copy link
Contributor Author

Ikuni17 commented Sep 27, 2023

Reviewers

We merged this to unblock the pipeline checks. Feel free to comment and suggest changes, and we will follow up.

ashokaditya
ashokaditya reviewed Sep 28, 2023
View reviewed changes
x-pack/test/fleet_api_integration/apis/fleet_telemetry.ts
expectedAgentCount: number,
attempts: number,
_attemptsMade = 0
): Promise<any> {
Copy link
Member

Choose a reason for hiding this comment

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

This should be Promise<GetAgentsResponse> that should be imported from @kbn/fleet-plugin/common

Copy link
Contributor

@watson watson Sep 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the tip. I opened a PR to fix: #167488

@ashokaditya ashokaditya mentioned this pull request Sep 28, 2023
Closed
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Sep 28, 2023
@ashokaditya
fix check type commit diff error
24cd2ad 
refs elastic/pull/166813
refs elastic/pull/167343
@watson watson mentioned this pull request Sep 28, 2023
Merged
watson pushed a commit that referenced this pull request Sep 28, 2023
Fix TypeScript issue in x-pack/test (#167488)
b90b211 
Follow up to #167343

See:
#167343 (comment)
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 28, 2023
@Ikuni17
Cherry pick elastic#167343
55cbe6e
@Ikuni17 Ikuni17 deleted the fix/x-pack-test-types branch February 9, 2024 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbudz jbudz jbudz left review comments

@ashokaditya ashokaditya ashokaditya left review comments

@watson watson watson left review comments

@JordanSh JordanSh JordanSh approved these changes

@ymao1 ymao1 ymao1 approved these changes

@delanni delanni delanni approved these changes

@sorenlouv sorenlouv sorenlouv approved these changes

@dasansol92 dasansol92 Awaiting requested review from dasansol92 dasansol92 was automatically assigned from elastic/security-defend-workflows

@parkiino parkiino Awaiting requested review from parkiino parkiino was automatically assigned from elastic/security-defend-workflows

Assignees

@delanni delanni

@Ikuni17 Ikuni17

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Fleet Team label for Observability Data Collection Fleet team Team:Operations Team label for Operations Team v8.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.