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

feature(ModalPage/Alert) Allow to pass testid to inner components #5792

Merged
merged 6 commits into from
Sep 15, 2023

Conversation

mendrew
Copy link
Contributor

@mendrew mendrew commented Sep 12, 2023

  • Unit-тесты
  • e2e-тесты (не нужны)
  • Дизайн-ревью (не требуется)

Описание

Основано на вопросах по поводу того как писать наиболее стабильные E2E тесты используя vkui.

Мотивация:

  • как можно меньше зависеть от внутренней реализации VKUI и уйти от селекторов в продуктовых E2E тестах.
  • не всегда удобно завязываться на текст, хоть и может быть предпочтительнее.

Почему не подождать подкомпонентного подхода #4699?

В подкомпонентном подходе это, конечно, будет легче сделать, но мы всё равно оставим для пользователей возможность использовать уже собранный вариант компонента по умолчанию и хотелось бы поддержать возможность для пользователей прокидывать внутрь data-testid.
В #2342 мы окончательно смогли бы решить эту проблему.

На данный момент просто хотелось бы дать возможность уже сейчас подготовиться пользователем к изменениям в css и отвязаться от классов в тестах.

Изменения

  • добавил проп modalContentTestId в ModalPage, чтобы можно было проще взаимодействовать с контейнером контента. Есть пара случаев где приходится в тестах контроллировать скролл. Поиск контейнера контента очень хрупок.
  • добавил тип HasDataAttribute в интервейс Alert actions, чтобы можно было прокидывать data-testid каждому экшену в Alert.
  • добавил тип HasDataAttribute в интервейс ModalPanelHeader/PanelHeader, чтобы можно было прокидывать data-testid.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 12, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit fcd82f1:

Sandbox Source
VKUI TypeScript Configuration

@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2023

size-limit report 📦

Path Size
JS 318.52 KB (+0.04% 🔺)
JS (gzip) 96.76 KB (+0.05% 🔺)
JS (brotli) 80.08 KB (+0.02% 🔺)
JS import Div (tree shaking) 2.95 KB (0%)
CSS 277.91 KB (0%)
CSS (gzip) 36.28 KB (0%)
CSS (brotli) 28.73 KB (0%)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2023

e2e tests

Playwright Report

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -1.42% ⚠️

Comparison is base (a3da40d) 81.99% compared to head (fcd82f1) 80.57%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5792      +/-   ##
==========================================
- Coverage   81.99%   80.57%   -1.42%     
==========================================
  Files         298      298              
  Lines        9839     9140     -699     
  Branches     3115     3113       -2     
==========================================
- Hits         8067     7365     -702     
- Misses       1772     1775       +3     
Flag Coverage Δ
unittests 80.57% <100.00%> (-1.42%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
packages/vkui/src/components/Alert/Alert.tsx 98.36% <ø> (-0.17%) ⬇️
...src/components/ModalPageHeader/ModalPageHeader.tsx 100.00% <ø> (ø)
...es/vkui/src/components/PanelHeader/PanelHeader.tsx 93.93% <ø> (-0.80%) ⬇️
...ckages/vkui/src/components/ModalPage/ModalPage.tsx 97.67% <100.00%> (-0.41%) ⬇️

... and 160 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2023

👀 Docs deployed

Commit fcd82f1

@mendrew mendrew marked this pull request as ready for review September 12, 2023 15:26
@mendrew mendrew requested a review from a team as a code owner September 12, 2023 15:26
Copy link
Contributor

@inomdzhon inomdzhon left a comment

Choose a reason for hiding this comment

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

Выглядит хорошо 👍

Есть, кстати, вот такая задача #2342

PS: есть не консистентность по неймингу, где-то будет формат modalContentTestId, а где-то data-testid, но с другой стороны ради консистентности добавлять дополнительные трансформации тоже не хочется, имею ввиду делать dataTestId, а потом это через проверку превращать в data-testid

@mendrew
Copy link
Contributor Author

mendrew commented Sep 14, 2023

Есть, кстати, вот такая задача #2342

Ага, я её в описании указал.

PS: есть не консистентность по неймингу, где-то будет формат modalContentTestId, а где-то data-testid, но с другой стороны ради консистентности добавлять дополнительные трансформации тоже не хочется, имею ввиду делать dataTestId, а потом это через проверку превращать в data-testid

Я бы тут не согласился называть это не консистентностью по неймингу. Тут всё же мы прокидываем testId дочернему компоненту, к которому вообще доступа нету.
Мы просто не можем дать ему dataTestId, потому что ModalPage уже сейчас можно прокинуть data-testid и он попадет на корневой элемент.

modalContentTestId в контексте ModalPage консистентент с getModalContentRef.

data-testid используется везде где это можно передать сразу через restProps в том или ином виде.

@mendrew mendrew self-assigned this Sep 14, 2023
@mendrew mendrew added the v5 Автоматизация: PR продублируется в ветку v5 label Sep 14, 2023
@mendrew mendrew requested a review from inomdzhon September 14, 2023 10:00
@inomdzhon inomdzhon added this to the v5.9.0 milestone Sep 14, 2023
inomdzhon
inomdzhon previously approved these changes Sep 14, 2023
Copy link
Contributor

@inomdzhon inomdzhon left a comment

Choose a reason for hiding this comment

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

👍

SevereCloud
SevereCloud previously approved these changes Sep 14, 2023
@mendrew mendrew dismissed stale reviews from SevereCloud and inomdzhon via fcd82f1 September 15, 2023 09:03
@mendrew mendrew merged commit 7a0fbb6 into master Sep 15, 2023
@mendrew mendrew deleted the mendrew/pass-test-id branch September 15, 2023 16:01
vkcom-publisher pushed a commit that referenced this pull request Sep 15, 2023
)

* Изменения
- добавил проп `modalContentTestId` в ModalPage, чтобы можно было проще взаимодействовать с контейнером контента. Есть пара случаев где приходится в тестах контроллировать скролл. Поиск контейнера контента очень хрупок.
- добавил тип `HasDataAttribute` в интервейс `Alert actions`, чтобы можно было прокидывать `data-testid` каждому экшену в `Alert`.
- добавил тип `HasDataAttribute` в интервейс ModalPanelHeader/PanelHeader,  чтобы можно было прокидывать `data-testid`.

Основано на вопросах по поводу того как писать наиболее стабильные E2E тесты используя vkui.

Мотивация:
- как можно меньше зависеть от внутренней реализации VKUI и уйти от селекторов в продуктовых E2E тестах.
- не всегда удобно завязываться на текст, хоть и может быть предпочтительнее.
@vkcom-publisher
Copy link
Contributor

v5.9.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v5 Автоматизация: PR продублируется в ветку v5
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants