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.

Sign up for GitHub

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

Jump to bottom

editoast: PacedTrain: add simulation summary and simulation endpoints #11022

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

editoast: PacedTrain: add simulation summary and simulation endpoints #11022

wants to merge 1 commit into from
+455 −160

Conversation

younesschrifi
Copy link
Contributor

@younesschrifi younesschrifi commented Mar 4, 2025 •
edited
Loading

Add simulation summary and simulation endpoints for paced trains.
Part of https://github.com/osrd-project/osrd-confidential/issues/779

@github-actions github-actions bot added the area:editoast Work on Editoast Service label Mar 4, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2025 •
edited
Loading

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

Codecov Report

Attention: Patch coverage is 92.57642% with 17 lines in your changes missing coverage. Please review.

Project coverage is 80.67%. Comparing base (28261be) to head (896daff).
Report is 6 commits behind head on dev.

Files with missing lines Patch % Lines
editoast/src/views/mod.rs 87.91% 11 Missing ⚠️
editoast/src/views/paced_train.rs 94.82% 6 Missing ⚠️

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

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev   #11022      +/-   ##
==========================================
+ Coverage   80.63%   80.67%   +0.03%     
==========================================
  Files        1101     1101              
  Lines      112360   112485     +125     
  Branches      747      747              
==========================================
+ Hits        90602    90744     +142     
+ Misses      21715    21698      -17     
  Partials       43       43
Flag Coverage Δ
editoast 72.37% <92.51%> (+0.17%) ⬆️
front 89.92% <100.00%> (ø)
gateway 2.18% <ø> (ø)
osrdyne 2.53% <ø> (ø)
railjson_generator 87.58% <ø> (ø)
tests 87.90% <ø> (ø)

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.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch from 1b17958 to 6d14fda Compare March 4, 2025 10:32
@github-actions github-actions bot added the area:front Work on Standard OSRD Interface modules label Mar 4, 2025
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch from 6d14fda to be809ed Compare March 4, 2025 10:33
@younesschrifi younesschrifi changed the title editoast: add simulation summary and simulation endpoints for paced t… editoast: PacedTrain: add simulation summary and simulation endpoints Mar 4, 2025
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch 4 times, most recently from 03cc77d to 422d56e Compare March 5, 2025 10:12
@younesschrifi younesschrifi marked this pull request as ready for review March 5, 2025 10:35
@younesschrifi younesschrifi requested review from a team as code owners March 5, 2025 10:35
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch 4 times, most recently from 73ffba7 to e44d339 Compare March 6, 2025 09:50
@younesschrifi younesschrifi requested a review from woshilapin March 6, 2025 09:56
leovalais
leovalais requested changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@leovalais leovalais left a comment

Choose a reason for hiding this comment

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

Thanks! Left a few comments, but nothing really blocking apart the ones about tests

@younesschrifi younesschrifi removed the request for review from woshilapin March 6, 2025 10:56
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch 2 times, most recently from 7c1cb3f to e1d6882 Compare March 6, 2025 14:28
woshilapin
woshilapin reviewed Mar 6, 2025
View reviewed changes
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch from e1d6882 to 13b13bc Compare March 6, 2025 15:17
@Wadjetz Wadjetz mentioned this pull request Mar 6, 2025
Open
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch from 13b13bc to 956a5d7 Compare March 6, 2025 15:43
This was referenced Mar 6, 2025
Open
Draft
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch 3 times, most recently from 161b369 to 014c70f Compare March 7, 2025 10:11
@younesschrifi younesschrifi requested a review from leovalais March 7, 2025 10:14
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch 7 times, most recently from 579cabf to cf54409 Compare March 7, 2025 14:19
woshilapin
woshilapin approved these changes Mar 7, 2025
View reviewed changes
Copy link
Contributor

@woshilapin woshilapin left a comment

Choose a reason for hiding this comment

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

Thanks for all the improvements. I left a bunch of nitpick comments.

@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch from cf54409 to 1b84707 Compare March 7, 2025 14:39
SharglutDev
SharglutDev approved these changes Mar 7, 2025
View reviewed changes
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 for front (not tested)

leovalais
leovalais approved these changes Mar 7, 2025
View reviewed changes
Copy link
Contributor

@leovalais leovalais left a comment

Choose a reason for hiding this comment

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

LGTM (not tested), a few last comments, thanks for the changes!

editoast/src/views/mod.rs Outdated
@@ -201,6 +203,45 @@ enum SimulationSummaryResult {
PathfindingInputError(PathfindingInputError),
}

fn simulation_response(sim: SimulationResponse) -> SimulationSummaryResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

impl From<core::simulation::SimulationResponse> for SimulationSummaryResult?

editoast/src/views/paced_train.rs
@@ -315,19 +399,35 @@ async fn project_path(

#[cfg(test)]
mod tests {
use axum::http::StatusCode;
use pretty_assertions::assert_eq;
Copy link
Contributor

Choose a reason for hiding this comment

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

:'(

It's useful for equality on deep structs like here

@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch 2 times, most recently from 03d6c19 to 989fc4d Compare March 7, 2025 16:41
@younesschrifi younesschrifi force-pushed the yci/add-simulation-paced-endpoint branch from 989fc4d to 896daff Compare March 7, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woshilapin woshilapin woshilapin approved these changes

@leovalais leovalais leovalais approved these changes

@SharglutDev SharglutDev SharglutDev approved these changes

Assignees

@younesschrifi younesschrifi

Labels
area:editoast Work on Editoast Service area:front Work on Standard OSRD Interface modules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@younesschrifi @codecov-commenter @woshilapin @leovalais @SharglutDev