[integration tests] Don't spend 60s on almost every test to parse XHTML

[lqc]

I'm not sure whether it takes so long to validate the DTD or is it just a timeout on trying to download it, but surely 60s is far too long to find a single string in an HTML file.

All tests now take a similar amount of time (~14s on my laptop). Still kinda suspicious cause tests on my other plugin which does almost nothing run in about 5s, so it means Spotbugs does something for almost 10s on every tests (even if it's just cheking if "effort" param was passed).

PS. Sorry about the code duplication, but I don't think there is any other way that does not involve restructuring the project into a multi-module one.

[hazendaz]

@lqc I'm not seeing the value of this at the moment. The runs on travis took the exact same amount of time so removing parallel runs to me makes less sense. Also, these type of changes should be per the issue #1 be opened against findbugs maven plugin. If I am to accept this I would just turn around and push it back to the original as it has nothing to do with the current repo. Since there are a number of items in what you did, can you split them into separate PR's and show that it actually speeds up the response? I agree things run entirely too slow but we got this from 45 minutes per run to 16 minutes and your PR took 16 minutes. Show some real drop in time and I'll merge and will take the time to go back to original author. I do admit my machine pretty much lights up when trying to run the integration tests so I'm not dismissing anything here but want to make sure we bring things in pieces that help improve things as well as get those back to original repo.

I'm leaving this one open, you can as well for now but smaller improvements will be better. As to utf-8 issues, I suspect there was a reason to that but you would need to raise that with the original plugin.

[hazendaz]

I should also note, we are off the original plugin so anything merged there will come here as I'm managing the gap.

[lqc]

36 vs. 31 is not the same amount of time - it's 14% less. Also Travis is not exactly the most stable thing in the world in terms of build times. How is running tests in 4 JVMs better that in one (even if the wall-clock time is the same) ?

Also, the XmlSlurper invocation is just plain out wrong. I would use some decent HTML assertions, but any attempt to add a test dependencies to invoker-plugin result in https://issues.apache.org/jira/browse/MINVOKER-182 .

Lastly, I'm not sure I really want to take the hassle of porting all changes to the findbugs plugin. It's cumbersome work because all configuration parameters, maven properties, etc. have been renamed. And I can't work on original, because I can't test it with my code (which is the reason I'm doing it) - again because everything was renamed (even the maven goal).

I really want to help improve Maven experience for Spotbugs, but I don't believe it can be done with the original plugin.

[hazendaz]

Please separate PR then if you are pushing it on me to get original plugin fixed. Please look at commits. I'm working with original author. This change doesn't really do anything for spotbugs itself. I realize it's a merge conflict problem. I will handle it if you break this PR into only what is necessary per PR. That is exactly the one change. Jvm issue is separate and Travis is stable. I get identical times on my machine. I will test that separately or I'll just recreate the need here

Get Outlook for Androidhttps://aka.ms/ghei36

---

From: Łukasz Rekucki notifications@github.com Sent: Monday, October 16, 2017 6:37:41 PM To: spotbugs/spotbugs-maven-plugin Cc: Jeremy Landis; Comment Subject: Re: [spotbugs/spotbugs-maven-plugin] [integration tests] Don't spend 60s on almost every test to parse XHTML (#7)

36 vs. 31 is not the same amount of time - it's 14% less. Also Travis is not exactly the most stable thing in the world in terms of build times. How is running tests in 4 JVMs better that in one (even if the wall-clock time is the same) ?

Also, the XmlSlurper invocation is just plain out wrong. I would use some decent HTML assertions, but any attempt to add a test dependencies to invoker-plugin result in https://issues.apache.org/jira/browse/MINVOKER-182 .

Lastly, I'm not sure I really want to take the hassle of porting all changes to the findbugs plugin. It's cumbersome work because all configuration parameters, maven properties, etc. have been renamed. And I can't work on original, because I can't test it with my code (which is the reason I'm doing it) - again because everything was renamed (even the maven goal).

I really want to help improve Maven experience for Spotbugs, but I don't believe it can be done with the original plugin.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/pull/7#issuecomment-337064108, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AA7ho4WH-Xnx7f6AaBeB0xC52IAeGpeAks5ss9q1gaJpZM4P7P3j.

[hazendaz]

I'll take care of this

Get Outlook for Androidhttps://aka.ms/ghei36

---

From: Łukasz Rekucki notifications@github.com Sent: Monday, October 16, 2017 6:37:41 PM To: spotbugs/spotbugs-maven-plugin Cc: Jeremy Landis; Comment Subject: Re: [spotbugs/spotbugs-maven-plugin] [integration tests] Don't spend 60s on almost every test to parse XHTML (#7)

36 vs. 31 is not the same amount of time - it's 14% less. Also Travis is not exactly the most stable thing in the world in terms of build times. How is running tests in 4 JVMs better that in one (even if the wall-clock time is the same) ?

Also, the XmlSlurper invocation is just plain out wrong. I would use some decent HTML assertions, but any attempt to add a test dependencies to invoker-plugin result in https://issues.apache.org/jira/browse/MINVOKER-182 .

Lastly, I'm not sure I really want to take the hassle of porting all changes to the findbugs plugin. It's cumbersome work because all configuration parameters, maven properties, etc. have been renamed. And I can't work on original, because I can't test it with my code (which is the reason I'm doing it) - again because everything was renamed (even the maven goal).

I really want to help improve Maven experience for Spotbugs, but I don't believe it can be done with the original plugin.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/pull/7#issuecomment-337064108, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AA7ho4WH-Xnx7f6AaBeB0xC52IAeGpeAks5ss9q1gaJpZM4P7P3j.

[hazendaz]

@lqc Please keep this open for time being until I'm done getting this on original plugin and back apply here. This took all of about 15 minutes to apply the bulk of your change against findbugs using simple search and replace. I then line breaked it to match the styling of the library rather than long lines for the setup. I still think invoker running parallel threads is needed. The time on my machine reduced from 16 minutes to 6 minutes by keeping it. I will test with removal and/or other adjustments plus look at what you note as still taking too long but want that to be separated out so we can get this back to @gleclaire project where the core of the issue started.

[hazendaz]

Please note the build time the PR took -> https://travis-ci.org/spotbugs/spotbugs-maven-plugin/builds/288770992?utm_source=github_status&utm_medium=notification

I will be confirming if leaving changes as soon as I push this up to findbugs-maven-plugin which will be in a couple of more minutes. I had miskeyed one file so running tests again ;)

[hazendaz]

It takes ~10 minutes with parallel run off. I'll look at adjusting it a bit to see if that makes any difference. That is on my machine. Travis is up next, pushing now.

[hazendaz]

travis takes 11/12 minutes with parallel on. It's still a great improvement there plus I'm still getting really fast speeds on my machine with it on. I'll test some more to see if there is a sweet stop less than 4 threads.

[lqc]

Thank you for your effort and I'm glad that you found this change useful after all. I'll try to push my other changes to the original plugin, although I still think that time spent on renaming things back-and-forth is a bit wasted.

[hazendaz]

Due to corporate environments, lack of developers performing proper hygiene, etc, I highly suspect findbugs-maven-plugin will continue being used for a long time. This is the only real reason I'm messing with fixing core issues there then bringing things down. I've got enough remotes setup that this isn't very difficult. Plus at the moment that seems to be the only place @gleclaire has remained and he has been providing additional useful fixes. The momentum is really good at the moment so I'm good with this route for time being.

[hazendaz]

@lqc Sorry that made a mess. I've rebased the upstream fixes for IT DTD issues into the code line now. I'll have to close this as it made it really messy. If you perform a rebase, there will be 29 files with merge conflict. As some of your original used a single line, I opted to split the lines for each. During the rebasing into spotbugs naming, it had some additional conflicts due to asserts one line above so I added some separating matching others.

As for timings, I tried a number of things yesterday including upgrading scm plugin to latest jgit for java 7. I'm going to send maven a pull request to get upgraded to that level as it does improve timgings a bit over a very old usage but not enough to matter much. I'm going to run some further tests with most recent to see if that helps any. A shallow clone would probably help quite a bit but that is still work in progress as of this month with jgit.