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

Reduce stack utilization of TestTemplateTestDescriptor::execute #4024

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

amaembo
Copy link
Contributor

@amaembo amaembo commented Sep 25, 2024

Overview

Fixes issue #4020


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

  • There are no TODOs left in the code
  • Method preconditions are checked and documented in the method's Javadoc -- no method declarations changed
  • Coding conventions (e.g. for logging) have been followed
  • Change is covered by automated tests including corner cases, errors, and exception handling -- refactoring only change, I believe no new tests are necessary
  • Public API has Javadoc and @API annotations -- no public APIs changed
  • Change is documented in the User Guide and Release Notes -- should I write something there, or the change is minor and doesn't worth mentioning?

@amaembo
Copy link
Contributor Author

amaembo commented Sep 25, 2024

Not sure about formatting. I've formatted the changed fragment using spotlessApply, even though it's inside the formatter:off section. Probably it worth removing formatter:off.

@jbduncan
Copy link
Contributor

Not sure about formatting. I've formatted the changed fragment using spotlessApply, even though it's inside the formatter:off section. Probably it worth removing formatter:off.

I think so too. From what I've observed of Spotless over the years, users like JUnit 5 use formatter:(off|on) because the bundled Eclipse formatter can't handle fluent chains like streams very well, so the comments are used as an escape hatch. Thus they should be safe to remove, if the team is happy of course. :)

@marcphilipp
Copy link
Member

Agreed, let's remove them

@marcphilipp marcphilipp self-assigned this Sep 25, 2024
@marcphilipp marcphilipp changed the title Reduce stack utilization by getting rid of outer Stream API call in TestTemplateTestDescriptor::execute Reduce stack utilization of TestTemplateTestDescriptor::execute Sep 25, 2024
@marcphilipp marcphilipp merged commit 8491772 into junit-team:main Sep 25, 2024
15 checks passed
@amaembo amaembo deleted the amaembo-patch-1 branch September 25, 2024 07:55
@marcphilipp
Copy link
Member

@amaembo Thank you for your contribution! I polished it a bit in c29dbb2.

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.

Reduce stack utilization by getting rid of outer Stream API call in TestTemplateTestDescriptor::execute
3 participants