Skip to content
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

Screenshot on fail does not include additional sessions #1266

Closed
GerroDen opened this issue Oct 18, 2018 · 1 comment
Closed

Screenshot on fail does not include additional sessions #1266

GerroDen opened this issue Oct 18, 2018 · 1 comment

Comments

@GerroDen
Copy link
Contributor

I use tests with an additional session. When the test fails i'd like to see a screenshot of that moment.

I only get a screenshot of the main session and none of the additional.

Feature with failing test inside a session:

Feature("Screenshots on fail in session");

Scenario("Failing in session", (I) => {
	session("new session", () => {
		I.amOnPage('http://google.com');
		I.dontSeeInCurrentUrl('http://google.com');
	});
});

Details

  • CodeceptJS version: 1.4.3
  • NodeJS Version: v10.12.0
  • Operating System: Ubuntu 16.04
  • Puppeteer: 1.9.0
@KMKoushik
Copy link
Collaborator

Fix is merged

@ChexWarrior ChexWarrior mentioned this issue Mar 13, 2020
31 tasks
ChexWarrior pushed a commit to ChexWarrior/CodeceptJS that referenced this issue Mar 13, 2020
I've realized upon further testing that my removal of the page
manipulation in saveScreenshot causes an old bug (codeceptjs#1266) to reappear.  I
believe the way to fix this may have to be inside saveScreenshot, for
that reason I'm reimplementing my original fix and we'll continue to
research the problem.
DavertMik pushed a commit that referenced this issue Mar 27, 2020
* Add config param to puppeteer session start

* Add sessionName to configuration

Automatically put sessionName in session configuration
if it already hasn't been passed in

* Check config is defined before adding sessionName

* Revert "Check config is defined before adding sessionName"

This reverts commit 145dfb3.

* Revert "Add sessionName to configuration"

This reverts commit 0c99ad6.

* Revert "Add config param to puppeteer session start"

This reverts commit c122b40.

* Pass sessionName to session.start

* store new session page in map

* Add new activeSession property

* Set active session to new session

Set activeSession to new session when session starts and
store the page created for new session in sessionPages
property

* Store new session page in sessionPages property

* Unset activeSessionPage when finalizing session run

* Use current session page for screenshots

* Use stored page for setPage method

* Use screenshot method directly on active session page object

* Create Digest helper

Added a helper for helper tests that will create an MD5 digest from a
path to a file.  The point of this helper is to aid in determining if
screenshots taken in different sessions within Puppeteer are actually
different or the same.

* Make getMD5Digest static

* Add session, container and recorder to test

In order for a session to run properly the recorder and container
classes are required for the functionality to work as expected.

* Create Puppeteer helper using container

In order for the session() method to work a couple of things
needed to happen.

First we needed to start the recorder
so its isRunning property would be set to true and the functions
passed in session to recorder would actually run.

Secondly we in order for a session to be successfully created
we need to create the new Puppeteer helper using the container create
method this ensures the call to container.helpers() in session
will actually return something.

Finally we assign the constructed Puppeteer helper to I
so the tests still work properly

* Add output path for screenshots in test

* Add first test

* Remove active session and session pages properties

* Use default browser context to get original page

* Stop checking for session in saveScreenshot

Because the context of a newly created page for a session
is saved within the vars of that session we only need to
properly use the setPage method before saveScreenshot runs

* Remove sessionName as arg to session.start()

* Revert modifications to helper test

* Update DigestHelper to accept and return array

To make the screenshot test less noisy I modified the method on Digest
Helper to accept an array of file paths and return an array
of md5 digests of those files in the same order

* Create test for screenshot fix

This test ensures that screenshots taken of different pages in
different sessions are in fact unique.  To verify this a digest
is created of each screenshot file and the following is done:

1. For screenshots taken of the same page in the same session we check
that the digests are equivalent.

2. For screenshots taken of a different page in different sessions we
check that the digests are not equivalent.

* Fix spelling of equivalent

* Use lowercase letter for start of scenario

* Add other helpers to test

* Remove page manipulation in saveScreenshot

The saveScreenshot method was grabbing the last created page
to use for a screenshot when it should rely on the current page.

* Add output path as env variable

* Add output path to yml definition

* set correct path for output directory

* Convert DigestHelper to an actual Helper

* Add DigestHelper to config

* Use async/await for returning digests

* Add DigestHelper to other config files

* Add DigestHelper to webdriver config

* Remove DigestHelper from non-applicable helpers

* Only apply test to Puppeteer and Playwright

* Add comments explaining tests

* Add output path to DigestHelper

* Use await for async call

* Use same path as screenshot dir for outputPath

* Cleanup screenshots taken during session test

The screenshot test could be tricked if the screenshots
taken in a previous run still exist so the DigestHelper
removes all screenshots from this test after all tests
finish.

* Fix output path for Playwright

* Rename DigestHelper

The helper is now cleaning up after the session screenshot
test and grabbing the output path so it does more than
just create digests of files, a name change seems
appropriate.

* Remove adding path to conainers and ci

* Readd original fix for Puppeteer

I've realized upon further testing that my removal of the page
manipulation in saveScreenshot causes an old bug (#1266) to reappear.  I
believe the way to fix this may have to be inside saveScreenshot, for
that reason I'm reimplementing my original fix and we'll continue to
research the problem.

* Clear active session when restoring original page

* Store page create for current session in helper

Similar to the fix used in Puppeteer.  To ensure we can later access
a page create for the session() method we store each page created
in a map with the key being the sessionName.

* Use page for current session capturing screen

* Pass session name when step fails to restoreVars

* Handle session passed to restoreVars

When a session is passed to restore vars instead of restoring
the default session we set the page to whatever page obj is stored
for that session in this.activeSessionName

* Pass current session name to restore vars

If a session name is present we will set the current page
to the page we've stored for that session instead of restoring
the default session

* Ensure page is set to a page from default context

Because of changes made to restoreVars its possible that
when the browser closes the current page (this.page) is set
to a page within in another context, to ensure we don't
get an error later we always set the current page to the first
from the default context.

* Ensure page is from default ctx on close

Because of changes made in _session.restoreVars its possible
when the Puppeteer helper calls _after that the current
page object isn't from the default context, this can lead
to issues so we always set the page to the first page in
the default ctx before closing other contexts

* Pass session name to restoreVars in async methods

* Check that this.page is set before changing

* Check that page is truthy before attempting to set

* Add name and config options to session start

* Set page to default session page when not restarted

There was an error occuring due to my attempt at cleaning up the session
for a page when the _after method was called.  If a browser was set to
restart it would first call setPage needlessly for the default session
and then call it again in the _closeBrowser() method which caused
issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants