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

front: add e2e tests deleting a rolling stock #7851

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

Maymanaf
Copy link
Contributor

Closes: #7774

@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 28.32%. Comparing base (dc11fc8) to head (d154d7e).

Files Patch % Lines
...s/RollingStockEditor/RollingStockEditorButtons.tsx 0.00% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #7851      +/-   ##
============================================
- Coverage     28.33%   28.32%   -0.01%     
  Complexity     2059     2059              
============================================
  Files          1270     1270              
  Lines        155543   155546       +3     
  Branches       3055     3055              
============================================
- Hits          44070    44059      -11     
- Misses       109649   109663      +14     
  Partials       1824     1824              
Flag Coverage Δ
core 75.00% <ø> (ø)
editoast 71.24% <ø> (-0.04%) ⬇️
front 9.95% <0.00%> (-0.01%) ⬇️
gateway 2.34% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 72.93% <ø> (ø)

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

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

@Maymanaf Maymanaf force-pushed the aba/e2e-delete-rolling-stock-test branch from 64ce2e6 to 7d23b16 Compare July 2, 2024 12:31
@Maymanaf Maymanaf marked this pull request as ready for review July 2, 2024 12:51
@Maymanaf Maymanaf requested a review from a team as a code owner July 2, 2024 12:51
@Maymanaf Maymanaf requested a review from kmer2016 July 3, 2024 13:09
@Akctarus
Copy link
Contributor

Akctarus commented Jul 3, 2024

With @RomainValls, we found that the test does not pass 100% of the time, because the "delete" button is not visible if the rolling stock name is too long. The bug is known and is being resolved

@Maymanaf Maymanaf force-pushed the aba/e2e-delete-rolling-stock-test branch from 7d23b16 to 61df8e4 Compare July 4, 2024 11:22
@Maymanaf Maymanaf force-pushed the aba/e2e-delete-rolling-stock-test branch from 61df8e4 to d154d7e Compare July 4, 2024 11:24
@Maymanaf
Copy link
Contributor Author

Maymanaf commented Jul 4, 2024

With @RomainValls, we found that the test does not pass 100% of the time, because the "delete" button is not visible if the rolling stock name is too long. The bug is known and is being resolved

I added a data-testidto directly locate the buttons, and limited the length of the generated name by UUID to 6 characters, avoiding this problem

Copy link
Contributor

@Akctarus Akctarus left a comment

Choose a reason for hiding this comment

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

All tests passed, LGTM

Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Keep getting this when trying to : npx playwright test tests/009-rollingstock-editor.spec.ts --ui --project=chromium. Am I missing something to launch the test ?

Capture d’écran 2024-07-05 à 08 57 32

Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Tested and it works nice, just left one question for the config.

front/playwright.config.ts Show resolved Hide resolved
Copy link
Contributor

@RomainValls RomainValls left a comment

Choose a reason for hiding this comment

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

LGTM

@Maymanaf Maymanaf requested review from SharglutDev and removed request for kmer2016 July 5, 2024 12:38
Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Lgtm and tested, good job !

@Maymanaf Maymanaf added this pull request to the merge queue Jul 5, 2024
Merged via the queue into dev with commit 0114e7c Jul 5, 2024
17 checks passed
@Maymanaf Maymanaf deleted the aba/e2e-delete-rolling-stock-test branch July 5, 2024 12:51
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.

front: add e2e tests deleting a rolling stock
5 participants