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

create screenshot dir if plugin is used (closes #4477) #4488

Merged
merged 3 commits into from Nov 15, 2019

Conversation

AlexKamaev
Copy link
Contributor

@AlexKamaev AlexKamaev commented Nov 14, 2019

I reverted the fix of #4303
Since it leaded to failure in custom browser provers
Now the dir will be created for all plugins but not for local browser.
For local browser the dir is created by browser-tools

@testcafe-build-bot
Copy link
Collaborator

@testcafe-build-bot
Copy link
Collaborator

@testcafe-build-bot
Copy link
Collaborator

@AlexKamaev AlexKamaev requested review from AndreyBelym and miherlosev and removed request for miherlosev and AndreyBelym November 15, 2019 08:36
@testcafe-build-bot
Copy link
Collaborator

@@ -317,8 +319,11 @@ export default class BrowserProvider {
else
await this._takeLocalBrowserScreenshot(browserId, screenshotPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we won't create a directory in the code path where _takeLocalBrowserScreenshot is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very unobvious. At least, we should add a comment about it.

But I would prefer to refactor the code to get both cases covered by makeDir.

@testcafe-build-bot
Copy link
Collaborator

@testcafe-build-bot
Copy link
Collaborator

@AlexKamaev
Copy link
Contributor Author

@testcafe-build-bot retest

@testcafe-build-bot
Copy link
Collaborator

@AlexKamaev
Copy link
Contributor Author

@testcafe-build-bot retest

@testcafe-build-bot
Copy link
Collaborator

@AlexKamaev AlexKamaev merged commit 81b5c5b into DevExpress:master Nov 15, 2019
kirovboris pushed a commit to kirovboris/testcafe-phoenix that referenced this pull request Dec 18, 2019
…Express#4488)

* create screenshot dir if plugin is used (closes DevExpress#4477)

* remove obsolete code

* refactoring
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.

None yet

4 participants