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

Fix train displaying as not honored despite respecting times & stop time #8699

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

kmer2016
Copy link
Contributor

@kmer2016 kmer2016 commented Sep 2, 2024

closes #8654

@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2024

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

Codecov Report

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

Project coverage is 36.90%. Comparing base (2817459) to head (4615d97).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
front/src/applications/operationalStudies/utils.ts 76.92% 0 Missing and 3 partials ⚠️

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

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #8699      +/-   ##
============================================
- Coverage     36.92%   36.90%   -0.02%     
  Complexity     2213     2213              
============================================
  Files          1255     1255              
  Lines        114109   114119      +10     
  Branches       3186     3188       +2     
============================================
- Hits          42135    42121      -14     
- Misses        70081    70103      +22     
- Partials       1893     1895       +2     
Flag Coverage Δ
core 74.75% <ø> (ø)
editoast 72.00% <ø> (-0.09%) ⬇️
front 14.92% <76.92%> (+<0.01%) ⬆️
gateway 2.20% <ø> (ø)
osrdyne 2.71% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 86.45% <ø> (ø)

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.

@kmer2016 kmer2016 force-pushed the cnh/fix-not-honored-train branch 2 times, most recently from beeef07 to 92f11e6 Compare September 2, 2024 16:41
@kmer2016 kmer2016 changed the title front: Fix train displaying as not honored despite respecting times &… front: Fix train displaying as not honored despite respecting times & stop time Sep 2, 2024
@kmer2016 kmer2016 changed the title front: Fix train displaying as not honored despite respecting times & stop time Fix train displaying as not honored despite respecting times & stop time Sep 2, 2024
@kmer2016 kmer2016 marked this pull request as ready for review September 2, 2024 16:45
@kmer2016 kmer2016 requested a review from a team as a code owner September 2, 2024 16:45
@kmer2016 kmer2016 force-pushed the cnh/fix-not-honored-train branch from 92f11e6 to 17a3acc Compare September 2, 2024 16:48
@Maymanaf Maymanaf requested review from Maymanaf and removed request for Maymanaf September 3, 2024 12:27
@Maymanaf
Copy link
Contributor

Maymanaf commented Sep 4, 2024

Works fine for me. Good job !

@Maymanaf Maymanaf self-requested a review September 4, 2024 08:27
Copy link
Contributor

@Caracol3 Caracol3 left a comment

Choose a reason for hiding this comment

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

Lgtm, good job 👌

Copy link
Contributor

@theocrsb theocrsb left a comment

Choose a reason for hiding this comment

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

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.

Lgtm

The bug was caused by attempting to match the schedule constraint data with the train's arrival times at each point along its route.
The first table (`trainSchedule.schedule`) contains only points with time constraints, while the second table (`trainSummary.path_item_times_final`) includes the train's arrival times for all intermediate points, including those without time constraints.
So, matching the indices between the two tables doesn't work because the first table skips points that aren't constrained.

front: improve complexity used to find index of schedule on path

front: add unit tests

- Add test for case when trainSchedule does not have a schedules array
- Add test for case when no matching index is found for a schedule on the path
@kmer2016 kmer2016 force-pushed the cnh/fix-not-honored-train branch from f4c2a00 to 4615d97 Compare September 5, 2024 07:05
@kmer2016 kmer2016 enabled auto-merge September 5, 2024 07:06
@kmer2016 kmer2016 added this pull request to the merge queue Sep 5, 2024
Merged via the queue into dev with commit 189d404 Sep 5, 2024
22 checks passed
@kmer2016 kmer2016 deleted the cnh/fix-not-honored-train branch September 5, 2024 07:30
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.

A train with respected times & stops is displayed "not honored"
7 participants