Re: RFR: 8244297: Provide utility for testing for memory leaks [v9]
On Sat, 24 Oct 2020 18:57:27 GMT, Kevin Rushforth wrote: >> Florian Kirmaier has updated the pull request incrementally with one >> additional commit since the last revision: >> >> JDK-8244297 >> Updated JMemoryBuddy based on codereview. > > Just a quick note, since I don't have time to review it today. Thanks for > making the changes. I see that you added the space after `if` (except in the > new method you added), but not after `for` and `while`. > > I'll do more testing next week as part of my review. Just added some more spaces! > modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 70: > >> 68: >> 69: if(folder1.exists()) return folder1.getAbsolutePath(); >> 70: if(folder2.exists()) return folder2.getAbsolutePath(); > > Space after `if` (I see you corrected most of them elsewhere in the code, > thanks). done - PR: https://git.openjdk.java.net/jfx/pull/204
Re: RFR: 8244297: Provide utility for testing for memory leaks [v9]
On Thu, 22 Oct 2020 19:50:23 GMT, Florian Kirmaier wrote: >> It's based on the discussion of my previous PR: >> https://github.com/openjdk/jfx/pull/71 >> >> I Added test utility class copied from JMemoryBuddy and used it to simplify >> 4 of the existing unit tests. >> >> It's a direct copy of my project >> [JMemoryBuddy](https://github.com/Sandec/JMemoryBuddy) without any changes. >> I'm also using it in most of the projects I'm involved with and in my >> experience, the tests with this Library are very stable. I can't remember >> wrong test results. Sometimes the memory behaviour of some libraries itself >> is not stable but the tests with JMemoryBuddy are basically always correct. > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > JDK-8244297 > Updated JMemoryBuddy based on codereview. Just a quick note, since I don't have time to review it today. Thanks for making the changes. I see that you added the space after `if` (except in the new method you added), but not after `for` and `while`. I'll do more testing next week as part of my review. modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 70: > 68: > 69: if(folder1.exists()) return folder1.getAbsolutePath(); > 70: if(folder2.exists()) return folder2.getAbsolutePath(); Space after `if` (I see you corrected most of them elsewhere in the code, thanks). - PR: https://git.openjdk.java.net/jfx/pull/204
Re: RFR: 8244297: Provide utility for testing for memory leaks [v9]
> It's based on the discussion of my previous PR: > https://github.com/openjdk/jfx/pull/71 > > I Added test utility class copied from JMemoryBuddy and used it to simplify 4 > of the existing unit tests. > > It's a direct copy of my project > [JMemoryBuddy](https://github.com/Sandec/JMemoryBuddy) without any changes. > I'm also using it in most of the projects I'm involved with and in my > experience, the tests with this Library are very stable. I can't remember > wrong test results. Sometimes the memory behaviour of some libraries itself > is not stable but the tests with JMemoryBuddy are basically always correct. Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision: JDK-8244297 Updated JMemoryBuddy based on codereview. - Changes: - all: https://git.openjdk.java.net/jfx/pull/204/files - new: https://git.openjdk.java.net/jfx/pull/204/files/3f731951..913622b9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=204&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=204&range=07-08 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jfx/pull/204.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/204/head:pull/204 PR: https://git.openjdk.java.net/jfx/pull/204
