Add unit tests for FilterValve, ProxyErrorReportValve and SemaphoreValve#976
Conversation
… and SemaphoreValve
ChristopherSchultz
left a comment
ChristopherSchultz
left a comment
There was a problem hiding this comment.
Some tests appear to be unnecessary.
|
|
||
|
|
||
| @Test | ||
| public void testInitParams() { |
There was a problem hiding this comment.
Is this (specific) test really necessary?
|
|
||
|
|
||
| @Test(expected = LifecycleException.class) | ||
| public void testInvalidFilterClassThrowsOnStart() throws Exception { |
There was a problem hiding this comment.
Is this (specific) test really necessary?
|
|
||
|
|
||
| @Test | ||
| public void testGetSetFilterClassName() { |
There was a problem hiding this comment.
Is this (specific) test really necessary?
| } | ||
|
|
||
| @Test(expected = IllegalStateException.class) | ||
| public void testGetServletContextThrowsBeforeStart() { |
There was a problem hiding this comment.
Is this (specific) test really necessary?
|
|
||
|
|
||
| @Test | ||
| public void testGetSetProperties() { |
There was a problem hiding this comment.
Is this (specific) test really necessary?
|
|
||
|
|
||
| @Test | ||
| public void testGetSetProperties() { |
There was a problem hiding this comment.
Is this (specific) test really necessary?
|
@ChristopherSchultz Those keep all coverage branches green without inflicting any issues. They can be used also as a resource for better understanding how the valves work. Since they are not resource heavy, I would keep them. |
Yeah, okay, I guess. |
3 participants
Summary
This PR improves test coverage by adding unit tests for several Valve
implementations that currently lack dedicated tests:
Motivation
While reviewing Tomcat valve implementations, these components were
identified as having little or no direct test coverage. Adding tests
helps improve reliability and prevents regressions during future
changes.
Changes
Added new test classes:
The tests cover:
Testing
All tests pass locally using:
Run all 3 new test classes
Run with coverage to verify coverage increase
No existing tests were modified.
Impact
No functional changes.
Test coverage improvement only.