Copilot commented on code in PR #62:
URL:
https://github.com/apache/sling-org-apache-sling-engine/pull/62#discussion_r2115526543
##########
src/test/java/org/apache/sling/engine/impl/SlingHttpServletResponseImplTest.java:
##########
@@ -107,24 +107,33 @@ public void testNoViolationChecksOnCommitedResponse() {
@Test
public void testReset() {
- final SlingHttpServletResponse orig =
mock(SlingHttpServletResponse.class);
+ final SlingHttpServletResponse originalResponse =
mock(SlingHttpServletResponse.class);
final RequestData requestData = mock(RequestData.class);
- final DispatchingInfo info = new
DispatchingInfo(DispatcherType.INCLUDE);
- when(requestData.getDispatchingInfo()).thenReturn(info);
- info.setProtectHeadersOnInclude(true);
-
- final HttpServletResponse include = new
SlingHttpServletResponseImpl(requestData, orig);
-
- when(orig.isCommitted()).thenReturn(false);
- include.reset();
- verify(orig, times(1)).isCommitted();
- Mockito.verifyNoMoreInteractions(orig);
-
- when(orig.isCommitted()).thenReturn(true);
- include.reset();
- verify(orig, times(2)).isCommitted();
- verify(orig, times(1)).reset();
- Mockito.verifyNoMoreInteractions(orig);
+ DispatchingInfo dispatchingInfo = new
DispatchingInfo(DispatcherType.INCLUDE);
+ when(requestData.getDispatchingInfo()).thenReturn(dispatchingInfo);
+ dispatchingInfo.setProtectHeadersOnInclude(true);
+
+ final HttpServletResponse includeResponse = new
SlingHttpServletResponseImpl(requestData, originalResponse);
+
+ when(originalResponse.isCommitted()).thenReturn(false);
+ includeResponse.reset();
+ verify(originalResponse, times(1)).isCommitted();
+ Mockito.verifyNoMoreInteractions(originalResponse);
+
+ when(originalResponse.isCommitted()).thenReturn(true);
+ includeResponse.reset();
+ verify(originalResponse, times(2)).isCommitted();
+ verify(originalResponse, times(1)).reset();
+ Mockito.verifyNoMoreInteractions(originalResponse);
+
+ // check if reset is called on error handling
+ dispatchingInfo = new DispatchingInfo(DispatcherType.ERROR);
+ when(requestData.getDispatchingInfo()).thenReturn(dispatchingInfo);
+ dispatchingInfo.setProtectHeadersOnInclude(true);
+ when(originalResponse.isCommitted()).thenReturn(false);
+ includeResponse.reset();
+ verify(originalResponse, times(2)).reset();
+ Mockito.verifyNoMoreInteractions(originalResponse);
Review Comment:
Multiple test scenarios in testReset rely on cumulative interactions on the
same mock; consider resetting the mock between scenarios or isolating tests to
avoid potential verification conflicts.
##########
src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java:
##########
@@ -90,7 +90,7 @@ public SlingHttpServletResponseImpl(RequestData requestData,
HttpServletResponse
}
}
- protected final RequestData getRequestData() {
+ public final RequestData getRequestData() {
Review Comment:
Changing the access modifier of getRequestData() from protected to public
may expose internal implementation details and affect the API contract; please
ensure this change is intentional and well documented.
--
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: dev-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
