GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Updated readInt method so streams are closed on failure or success by srnkan · Pull Request #1476 · processing/processing4

Conversation

srnkan commented Mar 11, 2026 •

edited by catilac

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

✅ Title: clear and descriptive

✅ Resolves: Resolves #1416

✅ Changes: I adapted the preexisting readInt method to include a try-with-resources, which closes all "closeables" regardless if the code errors or runs smoothly. This stops memory leakage by always closing necessary streams.

✅ Tests: Tests pass the "./gradlew test" and I did not add any additional tests

✅ Checklist: Tests pass the "./gradlew test" tests and branch is up-to-date

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Updated readInt so streams are closed

2dbe7b3

catilac closed this

Mar 11, 2026

catilac reopened this

Mar 11, 2026

srnkan commented Mar 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

I'd like @Ebaron96 to be assigned as a reviewer if possible

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

catilac commented Mar 11, 2026

Copy link

Copy Markdown

Collaborator

@srnkan sorry for the confusion in closing the issue without comment. i'll leave a review soon. i can't add ebaron96 as a reviewer until they have contributed code. but they are welcome to review.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

catilac self-requested a review

March 11, 2026 17:55

catilac requested changes

Mar 11, 2026

View reviewed changes

catilac left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is a great start! I noticed there aren't any tests in the UpdateCheckTest class.
It could use some!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

app/test/processing/app/UpdateCheckTest.java Outdated

Comment on lines +3 to +4

		public class UpdateCheckTest {
		}

catilac Mar 11, 2026

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This could use some tests!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

srnkan Mar 11, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I agree, but I was unable to compile tests for the issue because of the ongoing testing issue. Once that issue is resolved, I'm happy to go back and add tests. Thank you!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

catilac Mar 11, 2026

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

OH my gosh, yes. I just want to clarify my understanding of the testing issue, is it the broken tests for ProcessingPlugin ? Or something in addition to that?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ebaron96 Mar 11, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

looks good to me as well!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

srnkan Mar 11, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It's the same issue that Mark was talking about in the discord yesterday. Certain .gradlew tests fail automatically

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

catilac Mar 11, 2026

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ohh yes, thanks for clarifying @srnkan! that isn't a blocker in this case. we'll just assume those tests are broken, so feel free to get started on this test. it could be helpful to toss in a little assertTrue(true) as a sanity check

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

app/src/processing/app/UpdateCheck.java

Comment on lines +210 to +214

		try(InputStream stream = url.openStream();
		InputStreamReader isr = new InputStreamReader(stream);
		BufferedReader reader = new BufferedReader(isr);
		return Integer.parseInt(reader.readLine());
		BufferedReader reader = new BufferedReader(isr)) {
		return Integer.parseInt(reader.readLine());
		}

catilac Mar 11, 2026

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

this is really cool! good use of try-with-resources.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

marksherman reacted with rocket emoji

All reactions

🚀 1 reaction

srnkan Mar 23, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I added tests to the UpdateCheckTest.java file, with 16 tests. Also, Mark and I noticed that in the build.gradle.kts file under the app build, only kotlin tests are ran. So, I modified that file to include java tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          made tests for UpdateCheckTest.java and updated necessary files

b9b794e

srnkan force-pushed the ClosingStreamsIssue branch from 47a8802 to b9b794e Compare

March 23, 2026 18:27

catilac self-requested a review

July 8, 2026 05:21

catilac approved these changes

Jul 8, 2026

View reviewed changes

catilac left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

thanks for adding these tests!!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

catilac merged commit ab9a225 into processing:main

Jul 8, 2026

Reviewers

catilac

catilac approved these changes

+1 more reviewer

Ebaron96

Ebaron96 left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unclosed file streams in UpdateCheck

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

srnkan commented Mar 11, 2026 • edited by catilac Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srnkan commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

catilac commented Mar 11, 2026

Uh oh!

catilac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

catilac left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

srnkan commented Mar 11, 2026 •

edited by catilac

Loading

Uh oh!

There was an error while loading. Please reload this page.

srnkan commented Mar 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.