Grails
  1. Grails
  2. GRAILS-7992

MvcUnitTestCase reset() implementation - set committed=false before calling mockResponse.reset()

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.3.7
    • Fix Version/s: None
    • Component/s: Testing
    • Labels:
    • Environment:
      n/a
    • Testcase included:
      yes

      Description

      Should the implementation of MvcUnitTestCase.reset() set 'committed' to false BEFORE calling mockResponse?.reset() ?

      The current implementation of MvcUnitTestCase.reset() (from Grails v1.3.7) sets mockResponse.committed = false
      after calling mockResponse?.reset(), but the MockHttpServletResponse.reset() will throw IllegalStateException if committed == true.

      Here is the current implementation of reset() from MvcUnitTestCase:

      protected void reset() {
              mockRequest?.clearAttributes()
              mockRequest?.removeAllParameters()
              mockResponse?.reset()
              mockResponse?.committed = false
              mockSession?.clearAttributes()
              mockSession?.setNew(true)
      
              forwardArgs?.clear()
              redirectArgs?.clear()
              renderArgs?.clear()
              mockParams?.clear()
              mockFlash?.clear()
          }
      

      The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:

      public void reset() {
      		resetBuffer();
      		this.characterEncoding = null;
      		this.contentLength = 0;
      		this.contentType = null;
      		this.locale = null;
      		this.cookies.clear();
      		this.headers.clear();
      		this.status = HttpServletResponse.SC_OK;
      		this.errorMessage = null;
      	}
      
      public void resetBuffer() {
      		if (isCommitted()) {
      			throw new IllegalStateException("Cannot reset buffer - response is already committed");
      		}
      		this.content.reset();
      	}
      

      So it seems that if you're going to set 'committed = false' in MvcUnitTestCase.reset(), you should do it before calling mockResponse.reset().

      To recreate:
      We have a controller action that will return an HTTP 400 if required params are not provided, such as:

      if(!params.start || !params.end) {
            response.sendError(400, "missing.required.params.message")
            return
          }
      

      The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():

                            //first try without required 'start' and 'end' params
              controller.actionToTest()
      
              assertEquals 400, controller.response.status
              assertEquals "missing.required.params.message", controller.response.errorMessage
      
              //controller.response.committed = false
              reset()
      
              //now provide valid params
              controller.params.start = "1000"         
              controller.params.end = "5000"
      
              controller.actionToTest()
      

        Activity

        No work has yet been logged on this issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Shawn Flahave
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Last Reviewed:

              Development