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

Bugfix/#260 #269

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Bugfix/#260 #269

merged 5 commits into from
Jun 13, 2024

Conversation

d1z3d
Copy link
Contributor

@d1z3d d1z3d commented Jun 2, 2024

Добрый день!
Проблема заключается в том, что не удается заполнить данные при формировании страницы по шаблону workspace/wks-users (например, wrkInfo). В итоге возникает ошибка.
Изменения залил на https://hexlet-correction-ac3h.onrender.com

@fey
Copy link
Collaborator

fey commented Jun 4, 2024

@bazilval есть желание сделать код ревью? :_)

@fey fey requested a review from Malcom1986 June 4, 2024 13:04
@bazilval
Copy link
Collaborator

bazilval commented Jun 4, 2024

@fey гляну!

@bazilval
Copy link
Collaborator

bazilval commented Jun 8, 2024

@d1z3d привет!
Мне не очень нравится дублирование тут, весь блок повторяет код из метода getWorkspaceUsersPage()

Может как-то вынести это в какой-то общий метод, типа renderUserPage(), чтобы переиспользовать его и там, и там?

Ну и по мелочи: вот тут неудачное название для переменной, лучше так и назвать, что это ACCOUNT_INCORRECT_EMAIL, чтобы в тесте было понятно что мы подставили

@bazilval
Copy link
Collaborator

bazilval commented Jun 8, 2024

Ещё можно разметку заодно сделать поаккуратнее в части вывода ошибки:
image

Ну либо это можно уже в рамках #267 решить

@d1z3d
Copy link
Contributor Author

d1z3d commented Jun 9, 2024

@bazilval, привет!
Подготовку данных вынес в отдельный метод. Наименование сделал немного другое, но добавил java doc, чтобы было понятно для чего нужен метод.
Пофиксил отображение. По какой-то причине у параграфа был margin bottom в 1rem, поэтому параграф отображался криво. Изменения раскатил на render.
Снимок экрана 2024-06-09 в 18 24 05

@fey fey requested a review from bazilval June 10, 2024 13:27
Copy link
Collaborator

@bazilval bazilval left a comment

Choose a reason for hiding this comment

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

Вроде больше ни к чему не могу придраться

@d1z3d
Copy link
Contributor Author

d1z3d commented Jun 11, 2024

@bazilval, @fey, добрый день!
Вижу будет конфликт при мерже пр.
Мне необходимо внести правки? Здесь не очень представляю процесс, если такая проблема возникает.

UPD: поправил конфликт. Когда склонил проект, то выполнил команды по инструкции - https://ru.hexlet.io/blog/posts/Open-Source-github. По какой-то причине не подтянулись более свежие изменения.

@fey fey merged commit b4413bb into Hexlet:main Jun 13, 2024
@fey
Copy link
Collaborator

fey commented Jun 13, 2024

@d1z3d тесты упали, посмотрите. пожалуйста, в чем может быть дело.

@bazilval
Copy link
Collaborator

@d1z3d кавычки около емейла потеряли в тесте
Плюс надо убедиться, что там действительно значение емейла подставляется, а не что-то другое
Сейчас посмотрел на скрины ваши, там "The email "Email" is not valid"

Кстати, а как так вышло, на локальном тесты проходили или вы не проверяли?

@d1z3d
Copy link
Contributor Author

d1z3d commented Jun 13, 2024

@bazilval, @fey, добрый день!
Docker hub был заблокирован на тот момент. Выкачивал с амазона образ postgres + правил docker-compose, чтобы локально запустить проект, но в тестах используются тест контейнеры и не получилось их переделать для запуска тестов. Рассчитывал, что при билде на рендере будут запущены тесты и если будет что-то не так, то там поймаю ошибку, т.к. сервер находится не в России. На рендер деплой прошел без проблем.

@bazilval
Copy link
Collaborator

@d1z3d сделаете фикс?
Ну и по хорошему разобраться почему на рендере тесты прошли

@d1z3d
Copy link
Contributor Author

d1z3d commented Jun 14, 2024

@bazilval, добрый день!

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

Посмотрел лог на рендере, после build. Не происходит вызов тестов. В dockerfile билд происходит с флагом “-x test”. Он исключает выполнение тестов (https://www.baeldung.com/gradle-skip-tests). Поэтому у меня задеплоилось без ошибок. Хочу дополнительно проверить и пришлю лог.

@d1z3d
Copy link
Contributor Author

d1z3d commented Jun 15, 2024

Добрый день!
Кажется, что не получится отказаться от "-x test". По крайней мере, я не смог найти решение. Похожая проблема описана в этой статье на stackoverflow.

  • Проблема:
    Если в проекте используются тест контейнеры, то возникает проблема с доступами при монтировании файла "/sys/kernel/security". Раскатил локально и смотрел логи через Docker Desktop, использую DIND подход, потому что на бесплатном тарифе нельзя подключиться к серверу и посмотреть логи. Ошибка при деплое следующая (приложил ниже), не удается прочитать environment:
    #13 127.3 TypoReporterApplicationIT > initializationError FAILED #13 127.3 java.lang.IllegalStateException at DockerClientProviderStrategy.java:256 #13 129.3 #13 129.3 AccountRepositoryIT > initializationError FAILED #13 129.3 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.3 #13 129.3 AllowedUrlRepositoryIT > initializationError FAILED #13 129.3 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.3 #13 129.3 TypoRepositoryIT > initializationError FAILED #13 129.3 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.3 #13 129.3 WorkspaceRepositoryIT > initializationError FAILED #13 129.3 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.4 #13 129.4 WorkspaceRoleRepositoryIT > initializationError FAILED #13 129.4 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.4 #13 129.4 TypoServiceIT > initializationError FAILED #13 129.4 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.5 #13 129.5 WorkspaceServiceIT > initializationError FAILED #13 129.5 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.5 #13 129.5 WorkspaceSettingsServiceIT > initializationError FAILED #13 129.5 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.8 #13 129.8 AccountControllerIT > initializationError FAILED #13 129.8 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.8 #13 129.8 LoginIT > initializationError FAILED #13 129.8 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.8 #13 129.8 SignupControllerIT > initializationError FAILED #13 129.8 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.9 #13 129.9 TypoControllerIT > initializationError FAILED #13 129.9 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.9 #13 129.9 WorkspaceApiIT > initializationError FAILED #13 129.9 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 129.9 #13 129.9 WorkspaceControllerIT > initializationError FAILED #13 129.9 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212 #13 130.0 #13 130.0 WorkspaceSettingsControllerIT > initializationError FAILED #13 130.0 java.lang.IllegalStateException at DockerClientProviderStrategy.java:212

  • Решение через docker-compose:
    Если включить привилегированный режим, то тесты проходят через тест контейнеры, но толку от этого нет, т.к. render нужен dockerfile. Приложил docker-compose ():
    version: '3.7' services: test: image: docker:dind privileged: true volumes: - .:/app

@bazilval
Copy link
Collaborator

@d1z3d Да, отдельным пулл реквестом поправьте тест, чтобы деплой перестали падать

По поводу тест-контейнеров ничего не будем делать, печально, что не получается запустить локально и проверить сразу

@d1z3d
Copy link
Contributor Author

d1z3d commented Jun 15, 2024

@bazilval Добрый день!

Создал новый драфт пр, т.к. есть вопрос о необходимости изменении других dto - #278

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.

3 participants