[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-873987237 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-874450203 > Closing as not willing to read tons of text. Great reason, thank you so much. > I do not have time to read kilobytes of text for simple issues. Tibor, I am sorry to remind you that all this text would have been unnecessary if you would have read the shorter text in the original issue description correctly. Everything that followed was based on you misunderstanding it due to sloppy reading and first claiming the issue to be irrelevant, before promising to fix it. "You reap what you sow", they say. You are part of the problem, I was not talking to myself here but trying to explain things to you. > Improvements will be fixed as I promised. Thank you, looking forward to it. I promise that despite your dysfunctional and condescending behaviour here, I am still willing to help you re-test this, if you need support. Just notify me in new issues or PRs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-874045506 Of course you know better than me, which is why I sent my RFC about the open issue (zombie Java processes) in the first place. Now you finally have acknowledged that there actually **is** a problem. Before you * falsely claimed multiple times that this PR did not contain your previous bugfix, * called the back-ported reproducer test case in the other branch "irrelevant", * did not explicitly acknowledge that the zombie processes are a problem you are going to fix (just just acknowledged it now in your most recent 2 comments - thanks for that), * for no good reason copied my commits instead of merging this PR. Those things I was complaining and explaining about. Had you just acknowledged the problem before and somehow signalled that you are intending to fix them, either here or in a new issue/PR, it would have been fine. > 15 Well, that is nowhere to be found in your commits. Besides, I tried that before, both in the IT module settings and in the IT POMs themselves. I also tried the timeout in the IT tests themselves, all to no avail. If you see ways to improve this, I will be glad. BTW, would you please be so kind as to revert your commit (or force-push the previous HEAD) and merge this PR instead? You created your commit under the false notion of my PR not being forked off of master. I really would like to see my own commits here as a small recognition of my contribution. I spent lots of time, trying to explain this PR to you. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-873987237 What are you talking about? I don't wanted to write so much, but obviously didn't understand that this is a real problem and didn't care to investigate by yourself or acknowledge the zombie processes as a real problem. Instead, you criticised that I used an admin tool with a GUI instead of a console tool. We should care about this problem because none other than Maven creates them, they don't appear magically. When running the IT directly from its POM and pressing Ctrl-C, all child processes are terminated correctly. Only when running as part of the Surefire ITs, the zombie processes occur. How on earth can you **not** care and simply ignore an obvious, reproducible problem which is a huge system resource leak? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-873766789 I just found https://issues.apache.org/jira/browse/MSHARED-867 and left a comment. It could be related to the problem described here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-873764177 Please note my update at the end of https://github.com/apache/maven-surefire/pull/355#issuecomment-873703758 concerning differences in leftover zombie processes between cmd.exe and Git Bash. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-873712057 > Here is the [history ](https://github.com/kriegaex/maven-surefire/commits/before-SUREFIRE-1881)of commits in your repo - no fix! You don't get it, do you? This PR (branch `SUREFIRE-1881-fix` ) does contain the fixes! Branch `before-SUREFIRE-1881` is just for reproducing what would happen in case of a regression, because I needed a way to emulate that situation. What is so difficult to understand about it? This is why I named the other branch accordingly, with an explicit `before-` prefix in the branch name, and described it as a backport. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-873703758 > You did not take my commit with the fix. Your HEAD commit in your repository contains your tests without the fix. So it is quite irrelevant. What are you talking about?  You should read my PR correctly. The PR of course builds on top of your fixes, I branched it off of master. Please revert and merge my PR as prior art instead I did this work, you were simply recreating it. These are my commits and I want them merged. If something is wrong with my PR, we can fix them, of course. The other branch without the fixes simply shows what would happen if * we overlooked an endge case or * accidentally the problem gets re-introduced in the future by someone elses change or refactoring. This IT is meant to do two things: * verify some assumptions about how the application should behave * reproduce a problem, spotting regressions, in case it does not behave as expected So testing what would happen if the test actually would run into a timeout due to a regression is very much relevant. I simply created the other branch for your convenience, so you can reproduce the problem with hanging JVMs not being cleaned up properly. That other branch is not a PR branch, this one is! > Nobody is watching Windows task manager in order to send some proves because this is false representation. Of course not, normally people just run the IT, relying on it behaving correctly. Cleaning up resources and fixtures is part of a test's correct behaviour, and I manually verified that for you. I noticed that resource clean-up is broken, which is why it should be fixed, because otherwise people **not** looking at a task manager or `htop` would never notice what is going on, until all the running processes from repeatedly running tests would cause out of memory errors on their workstations or CI servers. > Instead, the developers use the command-line Different developers use different tools. If the tool reveals the correct information, why does it matter if I started it from a console or from an IDE? Besides, I did run the test from Maven(!), simply used a GUI tool to watch what Maven does live. In that tool, I cannot just see the end result but also what happens during the test: First, each of the two Java process groups are indirect children of sh.exe (my Git Bash), which starts the Maven build. Then, each time one of the two tests is interrupted due to the timeouts, the subprocesses lose their parent (of course) and now move up the process hierarchy, hanging there as zombies without parents. Your `wmic` shows the same information, only with less detail, because the fact that it is not just Java processes but a group of 4 processes per test case is missing. My simple screenshot illustrates that way better. You made a similar argument about my Process Explorer screenshot in the original Jira ticket, trying to fend off my information as irrelevant, claiming the Java agent to be the problem instead of Surefire. Like back then, you are also wrong in this case: The problem is real, my screenshot illustrates it you can easily reproduce it in my test branch. Of course, currently there is no known regression, but no software is perfect and regressions can always occur. To ensure that the ITs clean up their resources in case they fail due to a hanging JVM (which is exactly what they test should **not** happen), is part of the job of writing tests. So the job is not done yet. But if it makes you happy, I replayed again on the console for you (ran inside the IT module after a previous full build with install): ```none $ mvn -P run-its verify (...) [INFO] --- [INFO] T E S T S [INFO] --- [INFO] Running org.apache.maven.surefire.its.jiras.Surefire1881IT [ERROR] Tests run: 2, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 60.054 s <<< FAILURE! - in org.apache.maven.surefire.its.jiras.Surefire1881IT [ERROR] org.apache.maven.surefire.its.jiras.Surefire1881IT.testJVMLogging Time elapsed: 30.011 s <<< ERROR! org.junit.runners.model.TestTimedOutException: test timed out after 3 milliseconds at app//org.apache.maven.surefire.its.jiras.Surefire1881IT.testJVMLogging(Surefire1881IT.java:37) [ERROR] org.apache.maven.surefire.its.jiras.Surefire1881IT.testJavaAgent Time elapsed: 30.011 s <<< ERROR! org.junit.runners.model.TestTimedOutException: test timed out after 3 milliseconds at app//org.apache.maven.surefire.its.jiras.Surefire1881IT.testJavaAgent(Surefire1881IT.java:47) [INFO] [INFO] Results:
[GitHub] [maven-surefire] kriegaex commented on pull request #355: [SUREFIRE-1881] - Fix and extend integration test
kriegaex commented on pull request #355: URL: https://github.com/apache/maven-surefire/pull/355#issuecomment-873560441 According to the CI results, now `Surefire1881IT` is actually (and successfully for both cases) executed. So far, so good. Why in one case on MacOS `JUnit47ParallelIT.forcedShutdownVerifyingLogs` failed, I do not know, but I think I have seen it before. It looks like this test is flaky and should be fixed (in another PR). When looking at https://github.com/kriegaex/maven-surefire/actions, you see what happens in the back-ported tests, e.g. [here](https://github.com/kriegaex/maven-surefire/actions/runs/997851433). Of course, in the log there is no way to find out about the hanging processes which are never terminated, but for that you can look at my screenshot above or just run the test locally, keeping count of Java processes with a process manager. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
