-
Notifications
You must be signed in to change notification settings - Fork 44
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
editoast: fix max running time default value #9177
Conversation
4e578ef
to
cb1d0dc
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #9177 +/- ##
=========================================
Coverage 37.53% 37.54%
Complexity 2242 2242
=========================================
Files 1256 1257 +1
Lines 114937 114955 +18
Branches 3271 3272 +1
=========================================
+ Hits 43141 43156 +15
- Misses 69850 69852 +2
- Partials 1946 1947 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ! 👍🏽
cb1d0dc
to
eecc6ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for the PR's description, helped a lot 😅.
Would it make sense to add/modify some tests to avoid a regression on that topic?
eecc6ba
to
073ce34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix :)
Yes, most likely 🙈 It's a little tedious to properly test this, I've checked the parameters that are forwarded by core but it's not that easy to automate. But it should be doable with "planches travaux". I just need to figure out what's the proper way to write tests in editoast. I vaguely remember that the python integration tests are supposed to be replaced by the editoast test suite? |
073ce34
to
0bfa10d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
@woshilapin I've added an integration test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for adding a test (with beautiful ASCII art 😍).
Signed-off-by: Eloi Charpentier <eloi.charpentier.42@gmail.com>
0bfa10d
to
5647be6
Compare
Fix #9164
With some cleaning up
There was a misconception around the "max run time" parameter: it describes the maximum duration between the first train departure and its last arrival, stops included. But it was computed and used as if it were the duration between the earliest possible train departure and the latest possible arrival at the last stop.
(e.g. if we want the train to run for 2h max but leave at any time between 10:00AM and 10:00PM, we should set it to 2h, but we used to set it to 2+12h)