Add integration test for adding series to user's collection #525

olithin · 2017-01-04T22:39:50Z

Add integration test for adding series to user's collection

Addressed to #46

php-coder

I did a very high-level review. After removing most of the noise (like space related changes), I'll review this more detailed.

php-coder · 2017-01-09T11:47:19Z

src/test/java/ru/mystamps/web/tests/cases/WhenAdminAddSeries.java

@@ -114,7 +114,7 @@ public WhenAdminAddSeries() {

 	@BeforeClass
 	public void login() {
-		page.login(validAdminLogin, validAdminPassword);
+		page.login(validAdminPassword, validAdminLogin);


Looks strange.

php-coder · 2017-01-09T11:48:06Z

.travis.yml

 install: true

 # Clone repository with a depth of 1 commit.
 # https://docs.travis-ci.com/user/customizing-the-build/#Git-Clone-Depth
 git:
  depth: 1 # default: 50
+


.travis.yml file should be changed. Please, remove all these modifications or tell me why do we need them.

Need help here

php-coder · 2017-01-09T11:48:59Z

pom.xml

 	<modelVersion>4.0.0</modelVersion>
-	
+


Please, don't change spaces. You should modify only those lines that you need for the fix.

php-coder · 2017-01-09T11:49:43Z

src/main/java/ru/mystamps/web/controller/SeriesController.java

+
+			model.put("addSeriesSalesForm", new AddSeriesSalesForm());
+		}*/
+


This change should be removed.

php-coder · 2017-01-09T11:50:40Z

src/main/java/ru/mystamps/web/model/AddSeriesSalesForm.java

@@ -48,6 +53,7 @@
 	@NotNull
 	private Currency currency;

+


All changes from this file should be reverted. Again, try to reduce your modifications to minimum.

php-coder · 2017-01-09T11:53:29Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+		driver.get("http://127.0.0.1:8080/");
+		driver.findElement(By.linkText("1 item(s)")).click();
+		if (driver.findElements(By.cssSelector("input.btn.btn-danger")).size() != 0) {
+			System.out.println("Series is part of your collection");


Remove this System.out.println().

php-coder · 2017-01-09T11:54:05Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+public class StepDefinitions {
+	WebDriver driver = new FirefoxDriver();
+	@Given("^as a user$")
+	public void as_a_user() throws Exception {


Remove these throws Exception from all the methods. I believe that you don't need them.

php-coder · 2017-01-09T11:54:29Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+import org.openqa.selenium.firefox.FirefoxDriver;
+
+public class StepDefinitions {
+	WebDriver driver = new FirefoxDriver();


Member should be private

Let's use htmlunitdriver instead

Оба комментария все еще не исправлены.

Если я этот класс делаю приватным, то нет доступа к методам. Тест не проходит.

php-coder · 2017-01-09T11:54:58Z

src/test/resources/features/collection.feature

@@ -0,0 +1,9 @@
+Feature: User opens series info page, he should see a button for adding series to collection.
+
+  @in12


Remove tag.

php-coder · 2017-01-09T11:55:08Z

src/test/resources/features/collection.feature

+    Given as a user
+    When I add series to my collection
+    Then I am on the page with my collection
+    And I see this series have been added to the collection


Add a newline to the end of the file.

php-coder

Исправь пока то, что написал. Потом еще более детально посмотрю.

Автоматизировать это дело и приделывать к мавен плагину пока можно не делать, давай сначала код причешем.

php-coder · 2017-02-09T21:12:30Z

pom.xml

+			<artifactId>selenium-java</artifactId>
+		</dependency>
+
+


Убери здесь лишнюю строчку.

php-coder · 2017-02-09T21:12:50Z

pom.xml

@@ -41,6 +67,7 @@
 		</dependency>

 		<!-- https://github.com/spring-projects/spring-boot/blob/v1.4.3.RELEASE/spring-boot-starters/spring-boot-starter/pom.xml -->
+


Убрала красные, теперь нужно убрать лишние зеленые :)

php-coder · 2017-02-09T21:13:47Z

pom.xml

@@ -497,33 +524,43 @@

 		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.3.RELEASE/spring-boot-dependencies/pom.xml) -->
 		<selenium.version>2.53.1</selenium.version>
+		<cucumber-java.version>1.2.5</cucumber-java.version>
+		<cucumber-junit.version>1.2.5</cucumber-junit.version>


Давай объединим эти свойства в одно cucumber.version

php-coder · 2017-02-09T21:15:36Z

pom.xml

+		</dependency>
+		<dependency>
+			<groupId>org.seleniumhq.selenium</groupId>
+			<artifactId>selenium-java</artifactId>


Здесь нужно явно указать версию (используй selenium.version который уже есть в pom.xml)

php-coder · 2017-02-09T21:16:07Z

pom.xml

+			<artifactId>junit</artifactId>
+			<version>4.12</version>
+			<scope>test</scope>
+		</dependency>


Нужно отделить зависимости одной пустой строкой.

И не забудь добавить комментарий перед ней:


php-coder · 2017-02-09T21:26:25Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+
+	@Given("^as a user$")
+	public void as_a_user() {
+		driver.get("http://127.0.0.1:8080/account/auth");


Вместо адреса сайта и юрлов для страниц используй константы из класса Url.

php-coder · 2017-02-09T21:30:32Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+		driver.findElement(By.id("password")).click();
+		driver.findElement(By.id("password")).clear();
+		driver.findElement(By.id("password")).sendKeys("test");
+		driver.findElement(By.cssSelector("input.btn.btn-primary")).click();


При написании тестов старайся не использовать ничего кроме id. Если их нет, то будем добавлять. В данном конкретном случае, нужно найти форму по ее id и использовать метод submit().

См. также: http://stackoverflow.com/questions/17530104/selenium-webdriver-submit-vs-click

Этот комментарий все еще актуален.

php-coder · 2017-02-09T21:30:59Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+	public void as_a_user() {
+		driver.get("http://127.0.0.1:8080/account/auth");
+		driver.findElement(By.id("login")).click();
+		driver.findElement(By.id("login")).clear();


Зачем мы везде очищаем поля перед вводом текста? Без этого не работало?

php-coder · 2017-02-09T21:33:31Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+	@Then("^I see that this series have been added to the collection$")
+	public void i_see_that_this_series_have_been_added_to_the_collection() {
+		driver.get("http://127.0.0.1:8080/series/1");
+		Assert.assertTrue(driver.findElement(By.xpath(".//*[@class='btn btn-danger']//ancestor::form/p[1]")).getText().equals("Series is part of your collection"));


Используй статический импорт для assertTrue(). Посмотрев внимательнее. Используй assertEquals() вместо assertTrue() чтобы в случае падения теста было более вменяемое сообщение.

php-coder · 2017-02-09T21:34:10Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+
+	@Then("^I am on the page with my collection$")
+	public void i_am_on_the_page_with_my_collection() {
+		driver.get("http://127.0.0.1:8080/collection/coder");


Мне казалось, что здесь должна быть проверка того какая страница открыта, а не переход на эту страницу.

olithin

Ok

olithin · 2017-02-23T05:24:28Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+import org.openqa.selenium.firefox.FirefoxDriver;
+
+public class StepDefinitions {
+	WebDriver driver = new FirefoxDriver();


Если я этот класс делаю приватным, то нет доступа к методам. Тест не проходит.

php-coder

Добавил немного комментариев (внимательно пока не вглядывался, но для начала исправь хотя бы эти). Плюс необходимо исправить ошибки о которых сообщил бот.

php-coder · 2017-05-06T19:58:48Z

pom.xml

+			<scope>test</scope>
+		</dependency>
+
+		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.3.RELEASE/spring-boot-dependencies/pom.xml) -->


v1.4.3.RELEASE -> v1.4.6.RELEASE

Исправлено

php-coder · 2017-05-06T19:59:11Z

pom.xml


 		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.5.RELEASE/spring-boot-dependencies/pom.xml) -->
 		<servlet-api.version>3.1.0</servlet-api.version>

 		<skipUnitTests>false</skipUnitTests>
+


Все подобные добавления пустых строк нужно убрать.

Исправлено

php-coder · 2017-05-06T19:59:31Z

src/test/java/ru/mystamps/web/tests/CucumberRunner.java

+
+@RunWith(Cucumber.class)
+@CucumberOptions(features = "classpath:features")
+


Убери здесь пустую строку.

Исправлено

php-coder · 2017-05-06T20:00:27Z

src/test/java/ru/mystamps/web/tests/CucumberRunner.java

+@RunWith(Cucumber.class)
+@CucumberOptions(features = "classpath:features")
+
+public class CucumberRunner {


Добавь свое авторство с помощью javadoc комментария и тега @author (см. как это сделано в других файлах для примера).

Исправлено

php-coder · 2017-05-06T20:00:54Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+
+import static org.junit.Assert.assertEquals;
+
+public class StepDefinitions  {


Добавь комментарий со своим авторством.

php-coder · 2017-05-06T20:01:29Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+		driver.findElement(By.id("password")).click();
+		driver.findElement(By.id("password")).clear();
+		driver.findElement(By.id("password")).sendKeys("test");
+		driver.findElement(By.cssSelector("input.btn.btn-primary")).click();


Этот комментарий все еще актуален.

php-coder · 2017-05-06T20:01:50Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+
+	@Given("^as a user$")
+	public void loginAsUser() {
+		driver.get("http://127.0.0.1:8080/account/auth");


Используй константы из класса Url.

Исправлено

php-coder · 2017-05-06T20:02:27Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+			driver.findElement(By.linkText("1 item(s)")).click();
+		}
+		driver.findElement(By.cssSelector("input.btn.btn-success")).click();
+	}	


Добавь пустую строку после, чтобы отделить один метод от другого.

for locator - By.cssSelector("input.btn.btn-success") - I thinck we have to add id

Yes, please, add it.

php-coder · 2017-05-06T20:05:05Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+	@Then("^I see that this series have been added to the collection$")
+	public void i_see_that_this_series_have_been_added_to_the_collection() {
+		driver.get("http://127.0.0.1:8080/series/1");
+		String actual = driver.findElement(By.xpath(".//*[@class='btn btn-danger']//ancestor::form/p[1]")).getText();


Никаких xpath-ей не надо, просто добавь id к нужному элементу.

There is no id

But you can add it.

php-coder · 2017-05-06T20:05:34Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+	}
+
+	@Then("^I see that this series have been added to the collection$")
+	public void i_see_that_this_series_have_been_added_to_the_collection() {


Здесь и выше -- переименуй методы, чтобы использовать camelCase в именах.

olithin · 2017-05-11T06:13:55Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+	public void iAddSeriesToMyCollection() {
+		driver.get("http://127.0.0.1:8080/");
+		driver.findElement(By.linkText("1 item(s)")).click();
+		if(driver.findElements(By.cssSelector("input.btn.btn-danger")).size() != 0){


for locator - By.cssSelector("input.btn.btn-danger") - I thinck we have to add id

So, please, add it then.

php-coder · 2017-06-20T17:03:51Z

pom.xml

@@ -516,17 +546,21 @@
 		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-dependencies/pom.xml) -->
 		<slf4j.version>1.7.25</slf4j.version>

+


Why they're still here?

php-coder · 2017-06-20T17:10:54Z

pom.xml

+			<scope>test</scope>
+		</dependency>
+
+		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-dependencies/pom.xml) -->


This comment should be placed above the property when we're defining the version (<junit.version>4.12</junit.version>) instead of here.

php-coder · 2017-06-20T17:28:40Z

pom.xml

+		<dependency>
+			<groupId>org.seleniumhq.selenium</groupId>
+			<artifactId>selenium-java</artifactId>
+			<version>${selenium.version}</version>


We don't need this dependency because we already have selenium-api. Please, remove it.

php-coder · 2017-06-20T17:29:39Z

src/test/java/ru/mystamps/web/tests/CucumberRunner.java

+import cucumber.api.CucumberOptions;
+import cucumber.api.junit.Cucumber;
+import org.junit.runner.RunWith;
+/**


Please, add a blank new line between imports and a comment.

Поправь, если не лень.

php-coder · 2017-06-20T17:30:52Z

src/test/java/ru/mystamps/web/tests/CucumberRunner.java

+import cucumber.api.junit.Cucumber;
+import org.junit.runner.RunWith;
+/**
+ * @author Anna Osipova <qaosipova@gmail.com>


I don't recommend to specify email here. Also why you're using one email for authoring comments and another one here?

php-coder · 2017-06-20T17:31:36Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+import ru.mystamps.web.Url;
+
+import static org.junit.Assert.assertEquals;
+/**


Please, add a blank new line between imports and a comment.

Все-таки хорошо бы исправить.

php-coder · 2017-06-20T17:34:17Z

src/test/resources/features/collection.feature

+    Given as a user
+    When I add series to my collection
+    Then I am on the page with my collection
+    And I see that this series have been added to the collection


Need to replace have been by has been

php-coder · 2017-06-20T17:54:19Z

src/test/java/ru/mystamps/web/tests/StepDefinitions.java

+
+import static org.junit.Assert.assertEquals;
+/**
+ * @author Anna Osipova <qaosipova@gmail.com>


Please, remove e-mail from this tag and leave just a name/surname.

php-coder · 2017-06-20T17:59:35Z

src/test/resources/features/collection.feature

@@ -0,0 +1,8 @@
+Feature: User opens series info page, he should see a button for adding series to collection.


Let's rewrite it to User has a collection of stamps. He can add series of stamps to a collection.

php-coder · 2017-07-13T15:20:21Z

pom.xml

@@ -506,27 +527,29 @@
 		<selectizejs.version>0.12.3</selectizejs.version>

 		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-dependencies/pom.xml) -->
-		<selenium.version>2.53.1</selenium.version>


No, this line should be here because we need it.

php-coder · 2017-07-13T15:20:41Z

pom.xml


 		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-dependencies/pom.xml) -->
 		<servlet-api.version>3.1.0</servlet-api.version>

 		<skipUnitTests>false</skipUnitTests>

-		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-dependencies/pom.xml) -->
-		<slf4j.version>1.7.25</slf4j.version>


Why did you remove this? It should be here.

php-coder · 2017-07-13T15:21:09Z

pom.xml

 		<start-class>ru.mystamps.web.support.spring.boot.ApplicationBootstrap</start-class>

 		<subethasmtp.version>3.1.7</subethasmtp.version>
 		<surefire.plugin.version>2.19.1</surefire.plugin.version>
 		<testng.version>6.8.8</testng.version>
+


php-coder · 2017-07-13T15:21:22Z

pom.xml


 		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-dependencies/pom.xml) -->
 		<spock.version>1.0-groovy-2.0</spock.version>

 		<!-- Define default value for spring-boot-starter-parent (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-starters/spring-boot-starter-parent/pom.xml) -->
+


php-coder · 2017-07-13T15:21:34Z

pom.xml

 		<sortpom.plugin.version>2.5.0</sortpom.plugin.version>
+


mystamps-bot · 2017-07-13T15:26:55Z

	1 Warning
⚠️	danger check: branch `#46_cucumber_ganna` does not comply with our best practices. Branch name should use the following scheme: `ghXXX_meaningful-name` where `XXX` is an issue number. Next time, please, use this scheme :)

Generated by 🚫 Danger

php-coder · 2017-08-06T20:06:32Z

pom.xml


 		<subethasmtp.version>3.1.7</subethasmtp.version>
 		<surefire.plugin.version>2.19.1</surefire.plugin.version>
 		<testng.version>6.8.8</testng.version>
+


Хорошо бы удалить эту лишнюю строку.

php-coder · 2017-08-06T20:07:13Z

pom.xml

 		<!-- Define default value for spring-boot-starter-parent (https://github.com/spring-projects/spring-boot/blob/v1.4.7.RELEASE/spring-boot-starters/spring-boot-starter-parent/pom.xml) -->
-		<start-class>ru.mystamps.web.support.spring.boot.ApplicationBootstrap</start-class>
+
+    <start-class>ru.mystamps.web.support.spring.boot.ApplicationBootstrap</start-class>


А тут почему съехало оформление? Надо вернуть обратно.

php-coder

Что нужно еще сделать, чтобы этот pull request был смерджен:

(обязательно) исправить все ошибки от бота. См. его комментарий: Add integration test for adding series to user's collection #525 (comment)
(обязательно) объединить все коммиты в один
(очень желательно) исправить мелкие просьбы в комментариях о пробельных изменениях

Аня, осталось совсем чуть-чуть!

php-coder · 2017-09-19T18:48:57Z

Дай знать, если нужна моя помощь!

coveralls · 2017-09-25T21:31:00Z

Coverage remained the same at 77.84% when pulling f17132e on Goblic:#46_cucumber_ganna into 80b8ef1 on php-coder:master.

php-coder · 2017-09-25T22:45:05Z

Merged in f72c87d commit.

php-coder · 2017-09-25T22:51:19Z

Congratulations on your first commit! 👍 I've added you to our rating: https://github.com/php-coder/mystamps/wiki/team-members#closed-at-least-one-ticket :)

Now, I suggest to follow these steps to perform some cleanup: https://github.com/php-coder/mystamps/wiki/after_merge_steps#remove-merged-branch

olithin self-assigned this Jan 4, 2017

php-coder added the in progress label Jan 4, 2017

php-coder requested changes Jan 9, 2017

View reviewed changes

php-coder requested changes Feb 9, 2017

View reviewed changes

php-coder unassigned olithin Mar 14, 2017

php-coder force-pushed the master branch from 7dca59c to 1410b5a Compare April 5, 2017 22:27

olithin changed the title ~~#46 cucumber ganna~~ 46 cucumber ganna Apr 7, 2017

olithin commented Apr 7, 2017

View reviewed changes

php-coder requested changes May 6, 2017

View reviewed changes

olithin commented May 11, 2017

View reviewed changes

php-coder changed the title ~~46 cucumber ganna~~ Add integration test for adding series to user's collection Jun 20, 2017

Repository owner deleted a comment from mystamps-bot Jun 20, 2017

php-coder reviewed Jun 20, 2017

View reviewed changes

php-coder reviewed Jul 13, 2017

View reviewed changes

pom.xml Outdated

<sortpom.plugin.version>2.5.0</sortpom.plugin.version>

Copy link

Owner

php-coder Jul 13, 2017

Choose a reason for hiding this comment

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

:(

php-coder reviewed Aug 6, 2017

View reviewed changes

php-coder requested changes Aug 6, 2017

View reviewed changes

php-coder approved these changes Sep 25, 2017

View reviewed changes

commitChanges

f17132e

php-coder closed this Sep 25, 2017

php-coder added resolution/merged and removed in progress labels Sep 25, 2017


		model.put("addSeriesSalesForm", new AddSeriesSalesForm());
		}*/

		@@ -0,0 +1,9 @@
		Feature: User opens series info page, he should see a button for adding series to collection.

		@in12

		@@ -41,6 +67,7 @@
		</dependency>

		<!-- https://github.com/spring-projects/spring-boot/blob/v1.4.3.RELEASE/spring-boot-starters/spring-boot-starter/pom.xml -->


		@RunWith(Cucumber.class)
		@CucumberOptions(features = "classpath:features")


		import static org.junit.Assert.assertEquals;

		public class StepDefinitions {

		@@ -516,17 +546,21 @@
		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.6.RELEASE/spring-boot-dependencies/pom.xml) -->
		<slf4j.version>1.7.25</slf4j.version>

		@@ -0,0 +1,8 @@
		Feature: User opens series info page, he should see a button for adding series to collection.

Add integration test for adding series to user's collection #525

Add integration test for adding series to user's collection #525

Uh oh!

Conversation

olithin commented Jan 4, 2017 • edited by php-coder Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

php-coder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olithin Jan 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

php-coder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olithin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

php-coder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

olithin commented Jan 4, 2017 •

edited by php-coder

Loading

olithin Jan 17, 2017 •

edited

Loading