Grails

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

Details

  • Type: Improvement Improvement
  • Status: Open 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

Ascending order - Click to sort in descending order
Shawn Flahave made changes -
Field Original Value New Value
Status Open [ 1 ] Edit [ 10000 ]
Description Should the implementation of MvcUnitTestCase.reset() set 'committed=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.

To recreate:
We have a controller action that will return an HTTP 400 if required params are not provided, such as:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "Missing required params 'start' and 'end'")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //first try without required 'start' and 'end' params
        controller.actionToTest()

        assertEquals 400, controller.response.status
        assertEquals "Missing required params 'start' and 'end'", controller.response.errorMessage

        //controller.response.committed = false
        reset()

        //now provide valid params
        controller.params.start = "1000"
        controller.params.end = "5000"

        controller.actionToTest()
{code}
     		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.

The reset() implementation from MvcUnitTestCase:
{code}
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()
    }
{code}

The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:
{code}
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();
}
{code}

To recreate:
We have a controller action that will return an HTTP 400 if required params are not provided, such as:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "Missing required params 'start' and 'end'")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //first try without required 'start' and 'end' params
        controller.actionToTest()

        assertEquals 400, controller.response.status
        assertEquals "Missing required params 'start' and 'end'", controller.response.errorMessage

        //controller.response.committed = false
        reset()

        //now provide valid params
        controller.params.start = "1000"
        controller.params.end = "5000"

        controller.actionToTest()
{code}
    
Shawn Flahave made changes -
Status Edit [ 10000 ] Open [ 1 ]
Shawn Flahave made changes -
Status Open [ 1 ] Edit [ 10000 ]
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.

The reset() implementation from MvcUnitTestCase:
{code}
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()
    }
{code}

The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:
{code}
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();
}
{code}

To recreate:
We have a controller action that will return an HTTP 400 if required params are not provided, such as:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "Missing required params 'start' and 'end'")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //first try without required 'start' and 'end' params
        controller.actionToTest()

        assertEquals 400, controller.response.status
        assertEquals "Missing required params 'start' and 'end'", controller.response.errorMessage

        //controller.response.committed = false
        reset()

        //now provide valid params
        controller.params.start = "1000"
        controller.params.end = "5000"

        controller.actionToTest()
{code}
     		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:
{code}
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()
    }
{code}

The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:
{code}
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();
}
{code}

To recreate:
We have a controller action that will return an HTTP 400 if required params are not provided, such as:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "Missing required params 'start' and 'end'")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //first try without required 'start' and 'end' params
        controller.actionToTest()

        assertEquals 400, controller.response.status
        assertEquals "Missing required params 'start' and 'end'", controller.response.errorMessage

        //controller.response.committed = false
        reset()

        //now provide valid params
        controller.params.start = "1000"
        controller.params.end = "5000"

        controller.actionToTest()
{code}
    
Shawn Flahave made changes -
Status Edit [ 10000 ] Open [ 1 ]
Shawn Flahave made changes -
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:
{code}
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()
    }
{code}

The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:
{code}
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();
}
{code}

To recreate:
We have a controller action that will return an HTTP 400 if required params are not provided, such as:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "Missing required params 'start' and 'end'")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //first try without required 'start' and 'end' params
        controller.actionToTest()

        assertEquals 400, controller.response.status
        assertEquals "Missing required params 'start' and 'end'", controller.response.errorMessage

        //controller.response.committed = false
        reset()

        //now provide valid params
        controller.params.start = "1000"
        controller.params.end = "5000"

        controller.actionToTest()
{code}
     		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:
{code}
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()
    }
{code}

The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:
{code}
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();
}
{code}

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:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "Missing required params 'start' and 'end'")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //first try without required 'start' and 'end' params
        controller.actionToTest()

        assertEquals 400, controller.response.status
        assertEquals "Missing required params 'start' and 'end'", controller.response.errorMessage

        //controller.response.committed = false
        reset()

        //now provide valid params
        controller.params.start = "1000"
        controller.params.end = "5000"

        controller.actionToTest()
{code}
    
Status Open [ 1 ] Edit [ 10000 ]
Shawn Flahave made changes -
Status Edit [ 10000 ] Open [ 1 ]
Shawn Flahave made changes -
Status Open [ 1 ] Edit [ 10000 ]
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:
{code}
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()
    }
{code}

The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:
{code}
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();
}
{code}

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:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "Missing required params 'start' and 'end'")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //first try without required 'start' and 'end' params
        controller.actionToTest()

        assertEquals 400, controller.response.status
        assertEquals "Missing required params 'start' and 'end'", controller.response.errorMessage

        //controller.response.committed = false
        reset()

        //now provide valid params
        controller.params.start = "1000"
        controller.params.end = "5000"

        controller.actionToTest()
{code}
     		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:
{code}
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()
    }
{code}

The call to mockResponse?.reset() is handled by Spring's MockHttpServletResponse:
{code}
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();
}
{code}

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:
{code}
if(!params.start || !params.end) {
      response.sendError(400, "missing.required.params.message")
      return
    }
{code}

The following test code snippet will fail/error unless we explicitly set response.committed=false before calling reset():
{code}
                      //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()
{code}
    
Shawn Flahave made changes -
Status Edit [ 10000 ] Open [ 1 ]
Burt Beckwith made changes -
Workflow Grails2 [ 70067 ] jira [ 77645 ]
Burt Beckwith made changes -
Workflow jira [ 77645 ] Grails2 [ 86006 ]
Peter Ledbrook made changes -
Last Reviewed 01/Jan/10
Peter Ledbrook made changes -
Workflow Grails2 [ 86006 ] jira [ 89037 ]
Peter Ledbrook made changes -
Workflow jira [ 89037 ] Grails2 [ 97190 ]

People

  • Assignee:
    Unassigned
    Reporter:
    Shawn Flahave
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Last Reviewed: