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
Add evaluate() function #181
Changes from 5 commits
3e68fbe
572e8da
de3c887
9ec8cb4
51d6d1f
6a4cdfa
bbab732
694f1f8
55671cf
d33189d
a070843
47a6b6a
f04a950
baa7042
183d0b5
b758e9c
20bb805
77b1e2f
0bc942e
f9ed104
8d5b6e9
a0b5e65
0441da1
51e614e
90642b9
c1da6e7
5ce1663
817414f
e890c4f
451d0ce
c51da57
1003b5e
32438f6
bc963d3
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 |
---|---|---|
|
@@ -162,20 +162,6 @@ public function it_can_handle_a_permissions_error() | |
->save($targetPath); | ||
} | ||
|
||
/** @test */ | ||
public function it_can_use_the_methods_of_the_image_package() | ||
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 this test because method named |
||
{ | ||
$targetPath = __DIR__.'/temp/testScreenshot.jpg'; | ||
|
||
Browsershot::url('https://example.com') | ||
->format('jpg') | ||
->save($targetPath); | ||
|
||
$this->assertFileExists($targetPath); | ||
|
||
$this->assertMimeType('image/jpeg', $targetPath); | ||
} | ||
|
||
/** @test */ | ||
public function it_can_create_a_command_to_generate_a_screenshot() | ||
{ | ||
|
@@ -201,6 +187,7 @@ public function it_can_create_a_command_to_generate_a_screenshot() | |
'height' => 1080, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
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. Why is 'png' added in every test? Can this 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. It related to #180 This change is not related with this PR. Should I remove 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. I removed it |
||
], | ||
], $command); | ||
} | ||
|
@@ -228,6 +215,7 @@ public function it_can_create_a_command_to_generate_a_screenshot_and_omit_the_ba | |
'height' => 1080, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -451,6 +439,7 @@ public function it_can_use_given_user_agent() | |
], | ||
'userAgent' => 'my_special_snowflake', | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -473,6 +462,7 @@ public function it_can_set_emulate_media_option() | |
], | ||
'emulateMedia' => 'screen', | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -495,6 +485,7 @@ public function it_can_set_emulate_media_option_to_null() | |
], | ||
'emulateMedia' => null, | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -566,6 +557,7 @@ public function it_can_run_without_sandbox() | |
'args' => [ | ||
'--no-sandbox', | ||
], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -588,6 +580,7 @@ public function it_can_dismiss_dialogs() | |
'height' => 600, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -610,6 +603,7 @@ public function it_can_ignore_https_errors() | |
'height' => 600, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -631,6 +625,7 @@ public function it_can_use_a_proxy_server() | |
'height' => 600, | ||
], | ||
'args' => ['--proxy-server=1.2.3.4:8080'], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -658,6 +653,7 @@ public function it_can_set_arbitrary_options() | |
'baz' => 200, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -735,6 +731,7 @@ public function it_can_wait_until_network_idle() | |
'height' => 600, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
|
||
|
@@ -753,6 +750,7 @@ public function it_can_wait_until_network_idle() | |
'height' => 600, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -775,6 +773,7 @@ public function it_can_send_extra_http_headers() | |
'height' => 600, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
@@ -811,7 +810,22 @@ public function it_can_click_on_the_page() | |
'height' => 600, | ||
], | ||
'args' => [], | ||
'type' => 'png', | ||
], | ||
], $command); | ||
} | ||
|
||
/** @test */ | ||
public function it_can_set_type_of_screenshot() | ||
{ | ||
$targetPath = __DIR__.'/temp/testScreenshot.jpg'; | ||
|
||
Browsershot::url('https://example.com') | ||
->setScreenshotType('jpeg') | ||
->save($targetPath); | ||
|
||
$this->assertFileExists($targetPath); | ||
|
||
$this->assertMimeType('image/jpeg', $targetPath); | ||
} | ||
} |
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.
Can you refactor this, so we don't need an
else
statement?First thought: let's introduce a
getOutput
function. You can use an early return in that function.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.
Can you explain more about
getOutput
?You mean
request.action == 'getOutput'
?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.
I removed explicit base64 encoding code.
But I can't make it more simpler because passing
request.options
can't handleevaluate()
function which needs string argumentThere 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.
That
else
statement can be avoided by introducing a new functiongetOutput(request)
that returns the output.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.
I implemented it on d33189d
Is it the function you said?