OpenRailAssociation / osrd

An open source web application for railway infrastructure design, capacity analysis, timetabling and simulation
https://osrd.fr
GNU Lesser General Public License v3.0
419 stars 40 forks source link

editoast: refactor infra tests part 7 #7932

Closed hamz2a closed 5 days ago

hamz2a commented 1 week ago

Part of https://github.com/OpenRailAssociation/osrd/issues/6980

It's better to review the PR commit by commit. I created intermediate functions {function}_v2 to facilitate the migration, but I deleted everything at the end.

I also refactored persist_railjson to use DbConnection directly instead of DbConnectionPool. Consequently, I removed the macro persist! that was in there and futures::try_join!, and I added a transaction.

codecov-commenter commented 1 week ago

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

Codecov Report

Attention: Patch coverage is 82.45614% with 30 lines in your changes missing coverage. Please review.

Project coverage is 28.21%. Comparing base (bea5cee) to head (6b5fd5a). Report is 7 commits behind head on dev.

Files Patch % Lines
editoast/src/views/infra/edition.rs 59.37% 13 Missing :warning:
editoast/src/views/infra/mod.rs 66.66% 7 Missing :warning:
editoast/src/views/infra/railjson.rs 54.54% 5 Missing :warning:
editoast/src/views/infra/routes.rs 57.14% 3 Missing :warning:
editoast/src/views/infra/auto_fixes/mod.rs 77.77% 2 Missing :warning:

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #7932 +/- ## ============================================ - Coverage 28.25% 28.21% -0.05% Complexity 2075 2075 ============================================ Files 1276 1276 Lines 156193 156314 +121 Branches 3084 3084 ============================================ - Hits 44136 44102 -34 - Misses 110216 110371 +155 Partials 1841 1841 ``` | [Flag](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/7932/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [core](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/7932/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `75.03% <ø> (ø)` | | | [editoast](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/7932/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `70.79% <82.45%> (-0.23%)` | :arrow_down: | | [front](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/7932/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `9.91% <ø> (-0.01%)` | :arrow_down: | | [gateway](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/7932/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `2.34% <ø> (ø)` | | | [railjson_generator](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/7932/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `87.49% <ø> (ø)` | | | [tests](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/7932/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `72.93% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hamz2a commented 1 week ago

I'm curious about why we do need a lot of #[serial_test::serial] in commit 3040c34.

I added #[serial_test::serial] because I was getting deadlock errors.

woshilapin commented 1 week ago

I'm curious about why we do need a lot of #[serial_test::serial] in commit 3040c34.

I added #[serial_test::serial] because I was getting deadlock errors.

We dug a little bit to try to understand. It's a deadlock detected by the Postgresql instance itself. It might be due to concurrence on the generated id for infras since we do create very long transactions when working with infras in tests.

We did not find an obvious solution to work around the problem. Let's keep it like this.