Skip to content
This repository has been archived by the owner on Nov 15, 2020. It is now read-only.

3.0.1 snapshot #40

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

3.0.1 snapshot #40

wants to merge 9 commits into from

Conversation

AMDenisovSBT
Copy link

1.)Доработана аннотация ApiAction, теперь она повторяющаяся. То есть теперь мы можем на один и тот же класс вешать множество аннотаций ApiAction.

Пример:
@ApiAction(method = HTTP.POST, path = "corporate/product-selection/rest/v2/uploadScriptsFile", title = "загрузка скрипта c неверной кодировкой", template = "ErrorUploadScriptsFile_cod_UCS-2.json")
@ApiAction(method = HTTP.POST, path = "corporate/product-selection/rest/api/v1.0/uploadScriptsFile", title = "загрузка скрипта v1.0", template = "TM_commonRKO.csv")
@ApiAction(method = HTTP.POST, path = "corporate/product-selection/rest/v2/uploadScriptsFile", title = "загрузка скрипта", template = "testSubsystemCode_testGroupName_autotest1.0.json")
public class UploadScriptsFile extends CookieContainer{...}
2)Добавлена возможность запускать тесты из main. Добавлен пакет с классами src/main/java/cucumber/runtime/io/
Пример вызова:
String[] cucumberOptions = { "--glue", "ru.sbtqa.ufs.efs.api.stepdefinitions", "--no-dry-run", "--monochrome",
"classpath:src/test/resources/features", "--tags", tags, "--plugin",
"io.qameta.allure.cucumber2jvm.AllureCucumber2Jvm" };
CucumberStaticRunner.startTests(cucumberOptions);
3) В случае с тестами упакованными в jar, есть проверка на расположение template рядом с jar
4) Если Jar собирается при помощи плагина spring-boot-maven-plugin, то директория классов BOOT-INF/classes, стандартный ClassLoader не подходит. Доработан поиск через Property. Доработан класс ApiFactoryWrapper, метод getApiEntry

…теперь мы можем на один и тот же класс вешать множество аннотаций ApiAction.

Пример:
@ApiAction(method = HTTP.POST, path = "corporate/product-selection/rest/v2/uploadScriptsFile", title = "загрузка скрипта c неверной кодировкой", template = "ErrorUploadScriptsFile_cod_UCS-2.json")
@ApiAction(method = HTTP.POST, path = "corporate/product-selection/rest/api/v1.0/uploadScriptsFile", title = "загрузка скрипта v1.0", template = "TM_commonRKO.csv")
@ApiAction(method = HTTP.POST, path = "corporate/product-selection/rest/v2/uploadScriptsFile", title = "загрузка скрипта", template = "testSubsystemCode_testGroupName_autotest1.0.json")
public class UploadScriptsFile extends CookieContainer{...}
2)Добавлена возможность запускать тесты из main. Добавлен пакет с классами src/main/java/cucumber/runtime/io/
Пример вызова:
String[] cucumberOptions = { "--glue", "ru.sbtqa.ufs.efs.api.stepdefinitions", "--no-dry-run", "--monochrome",
                           "classpath:src/test/resources/features", "--tags", tags, "--plugin",
                           "io.qameta.allure.cucumber2jvm.AllureCucumber2Jvm" };
             CucumberStaticRunner.startTests(cucumberOptions);
3) В случае с тестами упакованными в jar, есть проверка на расположение template рядом с jar
4) Если Jar собирается при помощи плагина spring-boot-maven-plugin, то директория классов BOOT-INF/classes, стандартный ClassLoader не подходит. Доработан поиск через Property. Доработан класс ApiFactoryWrapper, метод getApiEntry
@0I0u3uk
Copy link
Contributor

0I0u3uk commented May 30, 2018

@AMDenisovSBT по поводу spring я так понял это сделано для того чтобы у нас в компании нормально девопс работал? Если да может это дело во внутреннию либу перенести ufsapi называется.


public class CucumberStaticRunner {

// private String status = "Finished: FAILURE";
Copy link
Contributor

Choose a reason for hiding this comment

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

Лучше убрать закомментированный код

public static final String CLASSPATH_SCHEME_TO_REPLACE = "classpath:";
private final ClasspathResourceLoader classpath;
private final FileResourceLoader fs;
public CustomMultiLoader(ClassLoader classLoader) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Нужно нормальное форматирование, newline и все такое

Copy link
Member

Choose a reason for hiding this comment

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

И сделать это нужно во всех новых файлах

try {
return springResource.getURL().toString();
} catch (IOException e1) {
e1.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Может сюда логирование подключить?

@@ -67,13 +79,13 @@ public ApiEntry getApiEntry(String title) throws ApiException {
if (null == currentEntry) {
currentEntry = getApiEntry(entriesPackage, title);

}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Лишние пробелы

Copy link
Member

@kosteman kosteman left a comment

Choose a reason for hiding this comment

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

Пока лишь проревью базовые вещи, не вникая в реализации.
Считаю что сначала мы должны решить, хотим ли мы получить спринг в зависимости. Мне не очень нравится эта идея. @clicman что думаешь?

pom.xml Outdated
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>ru.sbtqa.tag</groupId>
<artifactId>api-factory</artifactId>
<version>3.0-SNAPSHOT</version>
<version>3.0.1-SNAPSHOT</version>
Copy link
Member

Choose a reason for hiding this comment

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

В ветке менять версию не надо. Это автоматически произойдет при релизе

pom.xml Outdated
<artifactId>spring-core</artifactId>
<version>4.3.14.RELEASE</version>
<type>jar</type>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

стандартные отступ для xml - 2 пробела

public static final String CLASSPATH_SCHEME_TO_REPLACE = "classpath:";
private final ClasspathResourceLoader classpath;
private final FileResourceLoader fs;
public CustomMultiLoader(ClassLoader classLoader) {
Copy link
Member

Choose a reason for hiding this comment

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

И сделать это нужно во всех новых файлах

*
*/
//Повторяющаяся аннатация
Copy link
Member

Choose a reason for hiding this comment

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

Комментарий, тем более на русском с ошибкой:), здесь не нужен

if (runtime.getErrors().size() > 0 || runtime.getSnippets().size() > 0) {
status = "Finished: FAILURE";
}
System.out.println(status);
Copy link
Member

Choose a reason for hiding this comment

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

необходимо заменить все println и prinstacktrace на логирование

Copy link
Author

Choose a reason for hiding this comment

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

Тут нужно проверить. Добавил вывод в консоль на уровне warn

Output to the console

log4j.rootLogger=WARN
log4j.appender.AF=org.apache.log4j.ConsoleAppender
log4j.appender.AF.layout=org.apache.log4j.SimpleLayout
но на 100 % не уверен.
LOG.warn(status);
Тут обязательно необходимо выводить статус в консоль. Джоба будет по нему орентироваться.

@AMDenisovSBT
Copy link
Author

Да, spring я добавил что бы полноценно участвовать в DevOps-е. Так как одно из требований запускать тесты из jar. Пробовал JUnit-ом из main запускать, крайне не гибо получается, Нашел только такого плана решение, которое как мне кажется, очень удобное

ClassFinder classFinder = new ResourceLoaderClassFinder(resourceLoader, classLoader);
Runtime runtime = new Runtime(resourceLoader, classFinder, classLoader, runtimeOptions);
runtime.run();
if (runtime.getErrors().size() > 0 || runtime.getSnippets().size() > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Мне кажется, здесь мы можем использовать метод isEmpty() вместо size() > 0

public static final String CLASSPATH_SCHEME = "classpath*:";
public static final String CLASSPATH_SCHEME_TO_REPLACE = "classpath:";
private final ClasspathResourceLoader classpath;
private final FileResourceLoader fs;
Copy link
Contributor

Choose a reason for hiding this comment

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

Я думаю, стоит придумать для этой переменной более читаемое имя.

@@ -0,0 +1,38 @@
package cucumber.runtime.io;

import static java.util.Arrays.asList;
Copy link
Contributor

Choose a reason for hiding this comment

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

Мне кажется, этого статик импорта можно избежать.


private Iterable<Resource> convertToCucumberIterator(org.springframework.core.io.Resource[] resources) {
List<Resource> results = new ArrayList<Resource>();
for (org.springframework.core.io.Resource resource : resources) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Мне кажется, здесь возможен npe в случае возникновения исключения в блоке:
try {
resources = resolver.getResources(locationPattern);
} catch (IOException e) {
resources = null;
e.printStackTrace();
}
и последующего вызова метода convertToCucumberIterator(resources)

return gluePath.replace('/', '.').replace('\\', '.');
}

private static boolean isClasspathPath(String path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Мне кажется, стоит отсортировать методы по разделам: public -> protected -> private

Copy link
Member

Choose a reason for hiding this comment

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

Здесь не совсем согласен. Считаю, что порядок методов должен быть
resources
isClasspathPath
convertToCucumberIterator
packageName
stripClasspathPrefix

Основываюсь на том, что если располагать методы придерживаясь правила "Если внутри метода используются другие методы, то они должны быть расположены в порядке их вызовов", тогда код класса читается как "рассказ" и не нприходится листать листинг туда сюда в поисках реализации используемых методов.

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Лишняя строка

* the list of parameters
*/
public void fillParams(Map<String, String> params) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Лишняя строка

* @throws ru.sbtqa.tag.apifactory.exception.ApiException
* if there is an error in user's prepare steps
*/
public void prepare() throws ApiException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Почему у нас объявлено, что метод кидает ApiException, а по факту он этого не делает?

*/
public Object fire(String url) throws ApiException {
// Get request method of current api object
HTTP requestMethod = getActionMethod();
Copy link
Contributor

Choose a reason for hiding this comment

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

Мне кажется, вызов метода можно заменить на this.getActionMethod() и убрать комментарий выше


Map<String, String> hdrs = getHeaders();
switch (requestMethod) {
case GET:
Copy link
Contributor

Choose a reason for hiding this comment

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

проблемы с отступами

Copy link
Member

@clicman clicman left a comment

Choose a reason for hiding this comment

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

Давайте всю спринговую часть вынесем в ufs-api

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants