-
Notifications
You must be signed in to change notification settings - Fork 912
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
ARTEMIS-2408 Changing FileDescriptors rule as classRule #2762
Conversation
@brusdev I'm running tests with these changes in. We can't really extend the testsuite for 1.5 on a fast box, and 3 hours more on a slow box (that's what I'm seeing when I run the testsuite now). lets see how it goes with a classRule instead. perhaps it's what we need. |
@clebertsuconic the lsof check is very useful to investigate on files leaks so it could be kept only if maxVariance > 0 or it could be moved in NoFilesBehind (when filesToCheck = {}) or it could be moved in a new rule to enable only if necessary. |
ca035b1
to
edf9c2f
Compare
edf9c2f
to
cfff939
Compare
@brusdev: I'm a bit concerned about the lsof check, as being dead code.. the day you need it it hasn't been used forever and nobody knows what it does. say you and me retire in 10 years, or we decide to move to another project, people dealing with this codebase will have no clue. I changed it to show all the opened files in case there is a leak.. you as a developer would be able to figure out by looking at the list. I also changed the timeout on shutdown. The issue is that once you added the block shutdown the tests were taking a lot of time to process those. with this at 0, I could get back to previous times. If you could please review this? |
/** We let this to be defined as a System Variable, as we need a different timeout over our testsuite. | ||
* When running on a real server, this is the default we want. | ||
* When running on a test suite, we need it to be 0, You should see a property on the main pom.xml */ | ||
public static final int DEFAULT_SHUTDOWN_TIMEOUT = parseDefaultVariable("DEFAULT_SHUTDOWN_TIMEOUT", 30_000); |
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.
oops.. this is supposed to be 3 seconds
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.
fixed
cfff939
to
2d7d714
Compare
LGTM +1 |
No description provided.