-
Notifications
You must be signed in to change notification settings - Fork 425
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
Fixed isJansiConsoleInstalled performance issue #1976
Conversation
isJansiConsoleInstalled() was repeatedly called in a loop from the TextTable#toString method, which caused performance problems due to the constant calls to Class#forName. This change caches the result from isJansiConsoleInstalled so it doesn't repeatedly call Class#forName, which cannot change at runtime.
@ChrisTrenkamp I had to enable CI/CD for this PR, but now I see some tests are failing:
and
This may be due to the order in which tests are executed. |
Fixed unit test by resetting the cached isJansiConsoleInstalled result.
ModelTransformerTest#testUsage
The failing test cases are passing now, but it had to be done by resetting the cached value. Doesn't feel like the most elegant way to handle this. |
I see what you mean but I cannot think of an alternative either. 😅 |
Having some trouble with the CI/CD, but I do see this failing test:
|
Merged. Sorry, it took some time to sort out the CI/CD issues. |
Fixes #1975
isJansiConsoleInstalled() was repeatedly called
in a loop from the TextTable#toString method,
which caused performance problems due to the
constant calls to Class#forName. This change
caches the result from isJansiConsoleInstalled so
it doesn't repeatedly call Class#forName, which
cannot change at runtime.