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

core: add check on different constraints #2548

Merged
merged 25 commits into from
Dec 14, 2022

Conversation

majaziri
Copy link
Contributor

@majaziri majaziri commented Dec 9, 2022

closes #2486

@majaziri majaziri requested a review from a team December 9, 2022 13:09
@majaziri majaziri linked an issue Dec 9, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Dec 9, 2022

Codecov Report

Merging #2548 (311b128) into dev (44325fa) will increase coverage by 42.48%.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##                dev    #2548       +/-   ##
=============================================
+ Coverage     37.61%   80.09%   +42.48%     
- Complexity     1661     1666        +5     
=============================================
  Files           551      237      -314     
  Lines         16808     7410     -9398     
  Branches       2355      942     -1413     
=============================================
- Hits           6322     5935      -387     
+ Misses        10116     1136     -8980     
+ Partials        370      339       -31     
Flag Coverage Δ
core 83.44% <100.00%> (+0.05%) ⬆️
front ?

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

Impacted Files Coverage Δ
...srd/api/pathfinding/PathfindingRoutesEndpoint.java 89.79% <100.00%> (+1.84%) ⬆️
...ain/java/fr/sncf/osrd/utils/graph/Pathfinding.java 98.36% <100.00%> (+0.02%) ⬆️
...ts/Itinerary/DisplayItinerary/DisplayItinerary.tsx
...src/applications/osrd/views/OSRDSimulation/Map.tsx
...nt/src/common/BootstrapSNCF/ChipsSNCF/ChipsSNCF.js
front/src/common/Map/Layers/OperationalPoints.tsx
...tions/osrd/components/Simulation/drawGuideLines.js
...rc/applications/editor/tools/trackEdition/utils.ts
front/src/common/ButtonFullscreen.js
... and 307 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@Khoyo Khoyo left a comment

Choose a reason for hiding this comment

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

Please add tests for this new behavior.

Copy link
Contributor

@eckter eckter left a comment

Choose a reason for hiding this comment

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

Apart from the weird behavior in assertThrows, this looks good to me.

You should clean up the git history, the "fix" commits should be merged with the commit they fix. A simple way to do this is a git rebase -i dev, this opens a text editor and you can replace pick with fixup for fixup commits. This branch shouldn't have more than one or two commits. Then you need to git push --force-with-lease

@anisometropie anisometropie changed the title add check on different constraints core: add check on different constraints Dec 13, 2022
@anisometropie anisometropie requested a review from eckter December 13, 2022 18:35
@anisometropie anisometropie enabled auto-merge (squash) December 13, 2022 18:46
Copy link
Contributor

@Khoyo Khoyo left a comment

Choose a reason for hiding this comment

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

LGTM (modulo eckter's feedback re git history)

@anisometropie anisometropie merged commit 251eb73 into dev Dec 14, 2022
@anisometropie anisometropie deleted the mji/pathfinding-error-message branch December 14, 2022 08:59
Akctarus pushed a commit that referenced this pull request Dec 19, 2022
* add check on different constraints

* fix checkStyle

* fix additional checkStyle

* fix additional checkStyle

* add electrification & gauge throw tests

* correct behaviour when pathfinding is null

* correct behaviour when pathfinding is null

* fix compilation errors

* fix checkstyle errors

* correct error origin detection

* delete duplicated test + correct checkstyle errors

* formatting

* remove unecessary line

* Handle generic pathfinding error as well

* indentation

* simplify by making the constraints hashmap static

* make constraints private

* missing javadoc

* String format

* Better function flow (return early and the big error handling block at the end)

* simplify logic

* change assertThrows to assertEquels

* Fix generic case

* fix style

Co-authored-by: Valentin Chanas <[email protected]>
alexandredamiron pushed a commit that referenced this pull request Jan 17, 2023
* add check on different constraints

* fix checkStyle

* fix additional checkStyle

* fix additional checkStyle

* add electrification & gauge throw tests

* correct behaviour when pathfinding is null

* correct behaviour when pathfinding is null

* fix compilation errors

* fix checkstyle errors

* correct error origin detection

* delete duplicated test + correct checkstyle errors

* formatting

* remove unecessary line

* Handle generic pathfinding error as well

* indentation

* simplify by making the constraints hashmap static

* make constraints private

* missing javadoc

* String format

* Better function flow (return early and the big error handling block at the end)

* simplify logic

* change assertThrows to assertEquels

* Fix generic case

* fix style

Co-authored-by: Valentin Chanas <[email protected]>
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.

improve pathfinding error message
4 participants