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

[Reporting] Test cleanup fixes 2 #191122

Closed

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Aug 22, 2024

Part of #191676
Follows #190099

Reporting Functional tests and API integration tests potentially create degraded Task Manager metrics. These scenarios will unsuccessful task manager task:

  • If the data view is removed before the report job task is flushed
  • If the test user is removed before the report job is flushed

This PR seeks to improve Task Manager metrics for reporting jobs by being more careful in cleanup after tests. A good way to do this is to call await reportingAPI.expectAllJobsToFinishSuccessfully(reportPaths) in the after() hook before remove the user and test archive data.

@tsullivan tsullivan force-pushed the reporting/test-cleanup-fixes branch 2 times, most recently from f10e625 to 7985a77 Compare August 22, 2024 17:54
@tsullivan tsullivan force-pushed the reporting/test-cleanup-fixes branch from 7985a77 to 18437ba Compare August 22, 2024 18:06
@tsullivan
Copy link
Member Author

/ci

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

cc @tsullivan @eokoneyo

@@ -108,6 +108,8 @@ export const commonJobsRouteHandlerFactory = (
counters,
{ isInternal },
async (doc) => {
// FIXME look for potential report job task in task manager and remove that too
Copy link
Member Author

Choose a reason for hiding this comment

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

Handle this separately: #191551

@tsullivan
Copy link
Member Author

@eokoneyo hope you don't mind, I will work on finalizing this PR

@tsullivan
Copy link
Member Author

This needs to be merged into #192417.

@tsullivan tsullivan closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants