-
Notifications
You must be signed in to change notification settings - Fork 280
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
Session autostart #733
Session autostart #733
Changes from all commits
a78efd0
93087a2
d3a2a1b
cc3df3d
900e11a
dd2b7e0
d018f17
da11434
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,14 +57,32 @@ public function testIsStarted() | |
$this->assertTrue($this->session->isStarted()); | ||
} | ||
|
||
public function testStart() | ||
public function testStartWithoutRunningSession() | ||
{ | ||
$this->driver | ||
->expects($this->once()) | ||
->method('isStarted') | ||
->willReturn(false); | ||
|
||
$this->driver->expects($this->once()) | ||
->method('start'); | ||
|
||
$this->session->start(); | ||
} | ||
|
||
public function testStartWithRunningSession() | ||
{ | ||
$this->driver | ||
->expects($this->once()) | ||
->method('isStarted') | ||
->willReturn(true); | ||
|
||
$this->driver->expects($this->never()) | ||
->method('start'); | ||
|
||
$this->session->start(); | ||
} | ||
|
||
public function testStop() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this test must not be removed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the test as I added the tests for the start() method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, the test you kept describes the wrong interaction with the driver (it does not describe the usage of and I would keep the test ensuring we can visit without starting, as the test does not need to know that the code is calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the test ensures that start is called: public function testVisit()
{
$this->driver
->expects($this->once())
->method('start');
$this->driver
->expects($this->once())
->method('visit')
->with($url = 'some_url');
$this->session->visit($url);
} The test with isStarted is not contained in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But when you call PHPUnit mocks don't complain about incomplete interaction specifications, which is why your test is passing (and a reason why I actually prefer Prophecy, but I haven't converted the whole Mink testsuite to it) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But IMHO it doesn't matter in this case as we are finde if started() is called and afterwards the visit() method is called. Otherwise tests would need to be duplicated for the other autostart methods as well. If you want I can add this in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stof , @peterrehm , any updates? This seem to be last change to be made before merging. P.S. |
||
{ | ||
$this->driver->expects($this->once()) | ||
|
@@ -83,34 +101,10 @@ public function testRestart() | |
$this->session->restart(); | ||
} | ||
|
||
public function testVisitWithoutRunningSession() | ||
{ | ||
$this->driver | ||
->expects($this->once()) | ||
->method('isStarted') | ||
->willReturn(false); | ||
|
||
$this->driver | ||
->expects($this->once()) | ||
->method('start'); | ||
|
||
$this->driver | ||
->expects($this->once()) | ||
->method('visit') | ||
->with($url = 'some_url'); | ||
|
||
$this->session->visit($url); | ||
} | ||
|
||
public function testVisitWithRunningSession() | ||
public function testVisit() | ||
{ | ||
$this->driver | ||
->expects($this->once()) | ||
->method('isStarted') | ||
->willReturn(true); | ||
|
||
$this->driver | ||
->expects($this->never()) | ||
->method('start'); | ||
|
||
$this->driver | ||
|
@@ -322,6 +316,10 @@ public function testWait() | |
|
||
public function testResizeWindow() | ||
{ | ||
$this->driver | ||
->expects($this->once()) | ||
->method('start'); | ||
|
||
$this->driver->expects($this->once()) | ||
->method('resizeWindow') | ||
->with(800, 600, 'test'); | ||
|
@@ -331,6 +329,10 @@ public function testResizeWindow() | |
|
||
public function testMaximizeWindow() | ||
{ | ||
$this->driver | ||
->expects($this->once()) | ||
->method('start'); | ||
|
||
$this->driver->expects($this->once()) | ||
->method('maximizeWindow') | ||
->with('test'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically, these actions are not defined as supported before visiting (see the phpdoc of
start
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But @peterrehm is using them and they work perfectly. Otherwise there is no way to open window initially with given size so that upon page visiting we won't be resizing it again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the list of Methods i the phpdoc. Is this what you meant?