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

[Infrastructure] Wipe integration test resources and surface exceptions gracefully #1098

Closed
ghost opened this issue Jan 14, 2025 · 3 comments · Fixed by #1154
Closed

[Infrastructure] Wipe integration test resources and surface exceptions gracefully #1098

ghost opened this issue Jan 14, 2025 · 3 comments · Fixed by #1154
Assignees
Labels

Comments

@ghost
Copy link

ghost commented Jan 14, 2025

Currently many of our integration tests for our processors (text embedding, text image embedding, sparse encoding) follow the following pattern to create and cleanup resources

public void processor_integrationTest() {
    try {
        // load model
        // create pipeline/index
        // do tests/asserts
    } finally {
        // cleanup pipeline, index, model
    }
}

However, this causes issues like #1091 where if the integration tests fail in the try block before creating the pipeline, it will enter the finally block and attempt to delete the nonexistent pipeline. This causes an exception in the cleanup and masks the true exception, such as #1093 where the root issue is a memory circuit breaker trigger which is unclear from the test report.

We should rework our integration test cleanup so we can:

  • Log errors more clearly during resource cleanup to avoid confusion
  • Resource cleanup after every test rather than a part of every test, so cleanup won't directly cause test failures
@q-andy
Copy link

q-andy commented Jan 16, 2025

Hi, I switched GitHub accounts and deleted my other one, for questions about this issue you can contact me instead

@heemin32 heemin32 assigned q-andy and unassigned q-andy Jan 16, 2025
@heemin32 heemin32 added the good first issue Good for newcomers label Jan 16, 2025
@weijia-aws
Copy link
Contributor

I will take a look at this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
3 participants