Conversation
| class Browser: | ||
| def __init__(self): | ||
| self.driver = BrowserFactory.get_driver() | ||
| self.wait = WebDriverWait(self.driver, 10) |
| def get_driver(self): | ||
| return self.driver |
There was a problem hiding this comment.
можно для наглядности поле self.driver сделать protected
| return self.driver | ||
|
|
||
| def get(self, url): | ||
| self.logger.info(f"Переход по адресу: {url}") |
There was a problem hiding this comment.
Лучше в коде придерживаться английского языка + с русским языком потом можешь говна похавать в плане кодировок, но в современном мире это редкость уже
| def switch_to_the_tab(self, current_window_handle): | ||
| self.logger.info("Переключение на новую вкладку") | ||
| new_window_handle = None | ||
| for handle in self.driver.window_handles: | ||
| if handle != current_window_handle: | ||
| new_window_handle = handle | ||
| break | ||
| self.driver.switch_to.window(new_window_handle) |
There was a problem hiding this comment.
new_window_is_opened(current_handles)
Есть еще явное ожидание для открытия новой вкладки
| @staticmethod | ||
| def get_driver(): | ||
| driver = Chrome() |
There was a problem hiding this comment.
И прям вообще удалось без опций сконфигурировать? Ну давай хотя бы развер окна выставим чтобы он на любом окружении был всегда одинаковый. Если не выставлять размер браузера, то в headless режиме они могут быть очень очень маленькими по умолчанию и никакой элемент у тебя не будет находиться (почитай что такое --headless режим, если еще не знаешь)
| LOGIN = "admin" | ||
| PASS = "admin" |
There was a problem hiding this comment.
Не сказал бы что это секретные данные и их нужно как-то прятать. Видно, что они тестовые. Но давай с целью тренировки положим это в переменные окружения и почитай про переменные окружения, почему секретные данные кладут именно туда
| image_1 = image_sources[0] | ||
| image_2 = image_sources[1] | ||
| image_3 = image_sources[2] |
There was a problem hiding this comment.
image_1, image_2, image_3 = image_sources
Пользуйся распаковкой в таких места
| self.upload_image_page.upload_image2(path) | ||
|
|
||
| actual_name = self.upload_image_page.get_image_text() |
There was a problem hiding this comment.
Нет обработки диалогового окна системы
| @staticmethod | ||
| def setup_logger(name='framework_logger', log_file='framework.log', level=logging.DEBUG): |
There was a problem hiding this comment.
name, log_file, level - лучше вынести все в конфиг
| @staticmethod | ||
| def setup_logger(name='framework_logger', log_file='framework.log', level=logging.DEBUG): | ||
| logger = logging.getLogger(name) | ||
|
|
||
| if not logger.handlers: | ||
| logger.setLevel(level) | ||
| formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s') | ||
|
|
||
| file_handler = logging.FileHandler(log_file, encoding='utf-8') | ||
| file_handler.setFormatter(formatter) | ||
|
|
||
| console_handler = logging.StreamHandler() | ||
| console_handler.setFormatter(formatter) | ||
|
|
||
| logger.addHandler(file_handler) | ||
| logger.addHandler(console_handler) | ||
|
|
There was a problem hiding this comment.
кодировки, строки форматтера, файл, имя - все нужно вынести в конфиг
| self.wait.until( | ||
| lambda driver: len(driver.window_handles) > 1 | ||
| ) |
There was a problem hiding this comment.
Можно попробовать уже готовое явное ожидание
new_window_is_opened(current_handles)
| def find_element(self, by, value): | ||
| return self._driver.find_element(by, value) No newline at end of file |
There was a problem hiding this comment.
Этот метод не описываем, так как не используем. Если тебе "пришлось" его описать, значит ты допустил одну из популярных ошибок. Обрати внимание внимательно на то какой конкретно объект ты используешь в каждый конкретный момент времени (оригинальный или оберточный)
| @staticmethod | ||
| def get_driver(): | ||
| options = Options() | ||
| options.add_argument("--window-size=1920,1080") |
| self.driver = driver.get_driver() | ||
| self.locator = locator | ||
| self.description = description | ||
| self.wait = WebDriverWait(self.driver, 10) |
|
|
||
| class BaseElement: | ||
| def __init__(self, driver, locator, description=None): | ||
| self.driver = driver.get_driver() |
There was a problem hiding this comment.
Сохраняем мы везде оберточный драйвер, а уже когда нужно - получаем через него оригинальный
Если ты сохранишь оригинальный объект, то доступ к обертке утеряешь на протяжении всего класса BaseElement
|
|
||
|
|
||
|
|
|
|
||
| def get_all_paragraphs(self): | ||
| self.logger.info("Получение всех параграфов на странице") | ||
| inner_html = self.driver.execute_script(f'return document.querySelector("div.scroll").innerHTML;') |
There was a problem hiding this comment.
Ты правильно сделал что стал получать innerHTML, только ты переусложнил js-код
element.get_attribute("innerHTML") - найди элемент через явное ожидание и у него получи свойство, не нужно искать его через js
У элементов масса аттрибутов (не все они отображаются в html коде, некоторые скрыты, но они есть)
также есть схожее свойство - outherHTML, почитай разницу
| self.logger.info("Закрыли вкладку с заголовком: %s", self.NEW_WINDOW_TITLE) | ||
|
|
||
| browser.close_tab_by_title(self.NEW_WINDOW_TITLE) | ||
| self.logger.info("Закрыли вкладку с заголовком: %s", self.NEW_WINDOW_TITLE) |
There was a problem hiding this comment.
Можно сказать, что логи низкого уровня - это логи в твоем фреймворке. Средний уровень - page object, а самый верхний - тесты. Они вызывают друг друга как матрешка. Видеть 100 логов на разных уровнях к одному и тому же действию иногда тоже избыточно
| logger = Logger.setup_logger() | ||
|
|
||
|
|
||
| @pytest.mark.parametrize('path, image_name', [("C:\image.jpg", "image.jpg")]) |
There was a problem hiding this comment.
Абсолютные пути - очень жесткая ошибка. Используем только относительные, так как только они будут работать на разных системах
|
|
||
| self.actions_page = SliderPage(browser) | ||
| self.actions_page.wait_for_open() | ||
| slider_value = random.randint(0, 8) |
There was a problem hiding this comment.
0, 8 лучше вынести в константы или хотя бы переменные чтобы было нагляднее
| def new_window_is_opened(self, current_handles): | ||
| def _predicate(_driver): | ||
| return len(_driver.window_handles) > len(current_handles) | ||
| return _predicate No newline at end of file |
There was a problem hiding this comment.
Для этого есть готовое явное ожидание (правда оно еще переключается на новую вкладку)
Я не понимаю, зачем внутри вложенная функция и зачем наружу ее отдавать. Исходя из названия метода ты должен возвращать bool. Что-то перемедурил)
| # Используем явное ожидание для новой вкладки | ||
| self.wait.until(self.new_window_is_opened([current_window_handle])) |
There was a problem hiding this comment.
А готовое не подошло?
new_window_is_opened(current_handles)
| @property | ||
| def driver(self): | ||
| return self.driver_wrapper.get_driver() |
There was a problem hiding this comment.
Считаю что не совсем правильно получать оригинальный объект драйвера через BaseElement, я бы убрал этот метод. Этот метод должен быть у нашей обертки над браузером
Но логику ты понял, чтобы правильно отработало явное ожидание - ты должен передать в класс WebDriverWait оригинальный driver, а не твой оберточный
| def clear_input(self, input_field): | ||
| input_field.clear() No newline at end of file |
There was a problem hiding this comment.
Лучше переименовать в просто clear. Это нативное название метода (так он называется в оригинальном selenium). Плюс слово Input лишнее по причине того, что это метод объекта input (то есть то что это действие над input и так само собой разумеющееся)
| class Direction(Enum): | ||
| LEFT = 'left' | ||
| RIGHT = 'right' |
There was a problem hiding this comment.
Нужно сделать string enum. Почитай отличия от обычного
|
|
||
| def hover_over_figure(self, index): | ||
| self.logger.info(f"Наведение на фигуру с индексом {index}") | ||
| figure_template = (By.XPATH, self.FIGURE_TEMPLATE.format(index)) |
There was a problem hiding this comment.
Тут ты захардкодил By.XPATH внутри метода. А тип локатора должен быть всегда рядом с самим локатором. Тут или тебе нужно правильно воспользоваться кодом в инициализаторе BasePage (убрать локатор вовсе, он выставится XPATH автоматически) или указывать тип локатора вместе с форматируемой строкой и не хардкодить его в методе тогда
Локатор изменят - а нам придется искать где же там еще куски по методам еще разбросаны
| pyautogui.write(image) | ||
| pyautogui.press('enter') |
There was a problem hiding this comment.
Лучше это вынести в отднльную утилитку, например PyAutoGuiUtils или UploadFileUtils или еще как-то
|
|
||
| locator = (By.XPATH, self.PARAGRAPH.format(current_count)) |
There was a problem hiding this comment.
By.XPATH захардкожено. Читай коммент выше. Нужно поправить везде
|
|
||
| CHROME_OPTIONS = "--window-size=1920,1080" |
There was a problem hiding this comment.
Опции лучше хранить в списке, так как их часто добавляю новые
| class TestUploadImage: | ||
| logger = Logger.setup_logger() |
There was a problem hiding this comment.
Я конечно понимаю, что логгер сингтон и каждую инициализацию логгера ты получаешь все тот же объект. Думаю это не ошибка, но вот еще подход: можно передавать логгер через фикстуру, которая произведет инициализацию единожды
No description provided.