-
Notifications
You must be signed in to change notification settings - Fork 1
Classrename #9
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
Classrename #9
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package com.sawyereffect.steps; | ||
|
|
||
| import com.sawyereffect.util.DriverFactory; | ||
| import cucumber.api.Scenario; | ||
| import cucumber.api.java.After; | ||
| import cucumber.api.java.AfterStep; | ||
| import cucumber.api.java.Before; | ||
| import cucumber.api.java.BeforeStep; | ||
| import org.openqa.selenium.OutputType; | ||
| import org.openqa.selenium.TakesScreenshot; | ||
| import org.openqa.selenium.WebDriver; | ||
| import org.openqa.selenium.WebDriverException; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
|
|
||
| public class Hooks { | ||
| private static final String TAKE_SCREENSHOT = "Screenshot"; | ||
| private final Logger logger = LoggerFactory.getLogger(Hooks.class); | ||
| private DriverFactory driverFactory; | ||
| private WebDriver driver; | ||
|
|
||
| @Before | ||
| public void setupDriver() throws IOException { | ||
| driverFactory = new DriverFactory(); | ||
| driver = driverFactory.getDriver(); | ||
| } | ||
|
|
||
| @After | ||
| public void quitDriver(Scenario scenario) throws IOException { | ||
|
|
||
| if (driver != null && (scenario.isFailed() || scenario.getName().contains(TAKE_SCREENSHOT))) { | ||
|
|
||
| logger.debug("Taking screenshot of scenario {}", scenario.getName()); | ||
|
|
||
| try { | ||
| final byte[] screenshot = ((TakesScreenshot) driver).getScreenshotAs(OutputType.BYTES); | ||
| scenario.embed(screenshot, "image/png"); | ||
| } catch (WebDriverException e){ | ||
|
Member
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. 🎉 |
||
| logger.error("Not able to take screenshot", e); | ||
|
|
||
| } | ||
| } | ||
|
|
||
| if (driverFactory != null) { | ||
| driverFactory.destroyDriver(); | ||
| } | ||
| } | ||
|
|
||
| @BeforeStep | ||
| public void beforeStep(Scenario scenario) { | ||
| logger.debug("Executing before every step {}", scenario.getName()); | ||
| } | ||
|
|
||
| @AfterStep | ||
| public void afterStep(Scenario scenario) { | ||
| logger.debug("Executing after every step {}", scenario.getName()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,34 +2,20 @@ | |
|
|
||
| import com.sawyereffect.util.DriverFactory; | ||
| import com.sawyereffect.util.PropertyReader; | ||
| import cucumber.api.Scenario; | ||
| import cucumber.api.java.After; | ||
| import cucumber.api.java.AfterStep; | ||
| import cucumber.api.java.Before; | ||
| import cucumber.api.java.BeforeStep; | ||
| import cucumber.api.java.en.Given; | ||
| import org.openqa.selenium.*; | ||
| import org.openqa.selenium.support.ui.ExpectedConditions; | ||
| import org.openqa.selenium.support.ui.WebDriverWait; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| public class StartingSteps { | ||
| public class StartingSteps extends DriverFactory{ | ||
|
Member
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 do we need to extend DriverFactory?
Author
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. Since we don't create the driver here anymore, we need to extend to get the driver used in the execution.
Member
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 why it's better extending than creating the driver? My concern is that StartingSteps is not a DriverFactory and by creating a hierarchy we are indicating that.
Contributor
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. The way the framework was designed was so that Steps would be the ones extending DriverFactory to get all the needed objects preloaded (driver in this case) from a single point otherwise what would be needed is to change the architecture of the framework to use a delegation approach. That change is not part of this change and would require a different issue to be created so it could be accomplished there not on this pull request.
Member
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. Got it, thanks 👍 |
||
|
|
||
| private static final String TAKE_SCREENSHOT = "Screenshot"; | ||
| private final Logger logger = LoggerFactory.getLogger(StartingSteps.class); | ||
| private DriverFactory driverFactory; | ||
| private WebDriver driver; | ||
| private final PropertyReader reader = PropertyReader.getPropertyReader(); | ||
|
|
||
| @Before | ||
| public void setupDriver() throws IOException { | ||
| driverFactory = new DriverFactory(); | ||
| driver = driverFactory.getDriver(); | ||
| } | ||
| private final WebDriver driver = getDriver(); | ||
|
|
||
| @Given("^User is in google main page$") | ||
| public void user_is_on_home_page() throws Throwable { | ||
|
|
@@ -54,30 +40,4 @@ private void waitForPageLogoToLoad() { | |
| By.id("hplogo"))); | ||
| } | ||
|
|
||
|
|
||
| @After | ||
| public void quitDriver(Scenario scenario) throws IOException { | ||
|
|
||
| if (driver != null && (scenario.isFailed() || scenario.getName().contains(TAKE_SCREENSHOT))) { | ||
|
|
||
| logger.debug("Taking screenshot of scenario {}", scenario.getName()); | ||
|
|
||
| final byte[] screenshot = ((TakesScreenshot) driver).getScreenshotAs(OutputType.BYTES); | ||
| scenario.embed(screenshot, "image/png"); | ||
| } | ||
|
|
||
| if (driverFactory != null) { | ||
| driverFactory.destroyDriver(); | ||
| } | ||
| } | ||
|
|
||
| @BeforeStep | ||
| public void beforeStep(Scenario scenario) { | ||
| logger.debug("Executing before every step {}", scenario.getName()); | ||
| } | ||
|
|
||
| @AfterStep | ||
| public void afterStep(Scenario scenario) { | ||
| logger.debug("Executing after every step {}", scenario.getName()); | ||
| } | ||
| } | ||
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.
Where is this class being used?
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.
This class will be used to contain the Before and After Hooks.
So will be used every time a scenario run.
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.
Ok thanks. Probably it's a good idea to add documentation on how and why to use this methods.
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.
That information is already in Cucumber page, I don't see the need of duplicating the information in here.
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.
NVM, I was not clear of how this was working :) But @ulisespulido was kind enough to explain to me.