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

Work around legacy integration test resource leak. #4244

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Work around legacy integration test resource leak. #4244

merged 2 commits into from
Sep 18, 2024

Conversation

fisx
Copy link
Contributor

@fisx fisx commented Sep 17, 2024 •
edited
Loading

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 17, 2024
@fisx fisx force-pushed the fisx/work-around-legacy-integration-test-resource-leak branch from fd4c26e to 7c9b16e Compare September 17, 2024 20:34
@fisx fisx requested a review from pcapriotti September 17, 2024 20:38
pcapriotti
pcapriotti reviewed Sep 18, 2024
View reviewed changes
Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

Shouldn't we find the resource leak instead of patching it like this?

pcapriotti
pcapriotti approved these changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

I guess this is fine, since these tests are legacy and maybe they will go away at some point.

@fisx fisx force-pushed the fisx/work-around-legacy-integration-test-resource-leak branch from 022a333 to 2b95d6d Compare September 18, 2024 08:39
@fisx fisx merged commit a72c70a into develop Sep 18, 2024
9 checks passed
@fisx fisx deleted the fisx/work-around-legacy-integration-test-resource-leak branch September 18, 2024 12:40
@echoes-hq echoes-hq bot added the echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. label Sep 19, 2024
fisx added a commit that referenced this pull request Oct 9, 2024
@fisx
Revert "Work around legacy integration test resource leak. (#4244)"
bd423b2 
This reverts commit a72c70a.

(it turns out this only helps with resource consumption because
after running the first bach of tests, defaultMainWithIngredients
exits...  m|
fisx added a commit that referenced this pull request Oct 9, 2024
@fisx
Revert "Revert "Work around legacy integration test resource leak. (#…
1941f53 
…4244)""

This reverts commit bd423b2.
fisx added a commit that referenced this pull request Oct 9, 2024
@fisx
Revert "Work around legacy integration test resource leak. (#4244)"
d10487b 
This reverts commit a72c70a.

(it turns out this only helps with resource consumption because
after running the first bach of tests, defaultMainWithIngredients
exits...  m|
elland pushed a commit that referenced this pull request Oct 14, 2024
@fisx
[WPB-11472] Revert "Work around legacy integration test resource leak. (
54cdc26 
#4244)" (#4287)

* Revert "Work around legacy integration test resource leak. (#4244)"

This reverts commit a72c70a.

(it turns out this only helps with resource consumption because
after running the first bach of tests, defaultMainWithIngredients
exits...  m|

* hi ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pcapriotti pcapriotti pcapriotti approved these changes

Assignees
No one assigned
Labels
echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fisx @pcapriotti @zebot