Skip to content

Conversation

bubenkoff
Copy link
Member

@bubenkoff bubenkoff commented Jul 16, 2016

closes #64


This change is Reviewable

@coveralls
Copy link

coveralls commented Jul 16, 2016

Coverage Status

Coverage remained the same at 86.742% when pulling ab1b88b on escape-path-sep-for-screenshots into 1db679f on master.

@olegpidsadnyi
Copy link
Contributor

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


pytest_splinter/plugin.py, line 453 [r1] (raw file):

                screenshot_dir = os.path.join(splinter_screenshot_dir, classname)
                screenshot_file_name_format = '{0}.{{format}}'.format(
                    '{0}-{1}'.format(names[-1][:128 - len(name) - 5], name).replace(os.path.sep, '-')

linter fails here - line too long


Comments from Reviewable

@olegpidsadnyi
Copy link
Contributor

Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


tests/test_plugin.py, line 200 [r1] (raw file):

    """.format(simple_page_content), "-vl", "-r w")

    content = testdir.tmpdir.join('test_browser_screenshot_escaped', 'test_screenshot[escaped-param]-browser.html').read()

no, actually in this file


Comments from Reviewable

@bubenkoff bubenkoff force-pushed the escape-path-sep-for-screenshots branch from ab1b88b to 210da0d Compare July 16, 2016 17:01
@bubenkoff
Copy link
Member Author

Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions.


tests/test_plugin.py, line 200 [r1] (raw file):

Previously, olegpidsadnyi (Oleg Pidsadnyi) wrote…

no, actually in this file

done

Comments from Reviewable

@olegpidsadnyi
Copy link
Contributor

Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@bubenkoff
Copy link
Member Author

Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@olegpidsadnyi
Copy link
Contributor

Review status: all files reviewed at latest revision, 1 unresolved discussion.


pytest_splinter/plugin.py, line 453 [r1] (raw file):

Previously, olegpidsadnyi (Oleg Pidsadnyi) wrote…

linter fails here - line too long

OK

Comments from Reviewable

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.742% when pulling 210da0d on escape-path-sep-for-screenshots into 1db679f on master.

@olegpidsadnyi olegpidsadnyi merged commit bb7489f into master Jul 16, 2016
@bubenkoff bubenkoff deleted the escape-path-sep-for-screenshots branch November 3, 2016 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Taking screenshot fails if name of the test contains "/" character (using pytest-bdd)
3 participants