Skip to content

Stephans Models - #307

Closed
Lostlias wants to merge 17 commits into
mainfrom
test/stephans_models
Closed

Stephans Models#307
Lostlias wants to merge 17 commits into
mainfrom
test/stephans_models

Conversation

@Lostlias

@Lostlias Lostlias commented Jul 4, 2025

Copy link
Copy Markdown
Contributor

I added the .datadictionary, .dataflowdiagram and .json files for all of Stephan's Models.
Each Model has a corresponding Result file now and ExampleModelsTest.java was changed accordingly.
All Tests run through.

@Lostlias

Copy link
Copy Markdown
Contributor Author

This PR has all of Stephan's Models with constraint checking. They have been added to the existing structure.

@Lostlias Lostlias self-assigned this Jul 22, 2025
@Entenwilli

Copy link
Copy Markdown
Member

Additional formatting in 33307f7

@Entenwilli
Entenwilli self-requested a review July 22, 2025 11:41

@Entenwilli Entenwilli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a lot of new models (and test cases 🎉)

Some smaller notes:

  • Consistent naming of edges: Use snake_case
  • Consistent naming of nodes: Use Title Case (you can and should use spaces here!)
  • There seem to be some changes in the automatically generated code (from one commit), please revert or exclude that commit from this PR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this Model have two constraints?
The existing one is correct, but as I understand there should also be the constraint that only managers are allowed to access data from a different region (e.g. ManagerUSA -> Asia, but not ClerkUSA -> Asia)

@Nicolas-Boltz

Copy link
Copy Markdown
Member

@Entenwilli The current issue during the build seems to be something we have encountered before. Can you please look at it, so we can finally merge this PR? I think it should be high priority

@Entenwilli

Copy link
Copy Markdown
Member

@Entenwilli The current issue during the build seems to be something we have encountered before. Can you please look at it, so we can finally merge this PR? I think it should be high priority

The workflow fails in this case, because there are some violations missing and some that are not properly defined for the Private Taxi and Distance Tracker example model. From having a quick glance at the models, I would say the propagation results look correct, so the issue is most likely the constraint or the defined expected violations

@Lostlias

Lostlias commented Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

This PR is completely succeeded by #340 .

@Entenwilli Entenwilli closed this Nov 4, 2025
@BenjaminArp
BenjaminArp deleted the test/stephans_models branch May 20, 2026 14:24
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.

3 participants