Skip to content

Initialize CI for PullRequest Testing #4767

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

Open
MatthiasBSchaefer wants to merge 1 commit into
base: master
Choose a base branch
from
+35 −0
Open

Initialize CI for PullRequest Testing #4767

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .github/workflows/PRTesting.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: PR_testing
on:
# Triggers the workflow on push or pull request events but only for the "main" branch
pull_request:

# Allows you to run this workflow manually from the Actions tab
workflow_dispatch:

# A workflow run is made up of one or more jobs that can run sequentially or in parallel
Comment on lines +1 to +9
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
name: PR_testing
on:
# Triggers the workflow on push or pull request events but only for the "main" branch
pull_request:
# Allows you to run this workflow manually from the Actions tab
workflow_dispatch:
# A workflow run is made up of one or more jobs that can run sequentially or in parallel
name: CI ReSim
on:
pull_request:
workflow_dispatch:

jobs:
prepare:
runs-on: self-hosted
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
runs-on: self-hosted
runs-on: [ self-hosted, linux, regression-testing ]

steps:
- name: checkout_pr_repo
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
- name: checkout_pr_repo
- name: Checkout code

run: /home/resimuser/regression_testing/docker/work/run_scripts/prepare_pr.sh ${{ github.event.number }}
Copy link
Member

@beutlich beutlich Mar 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

For the sake of transparancy, can we add all the CI bash scripts to a new folder ReSim in .CI? Otherwise it will be pure magic for library officers. Usually and as best practice the CI scripts are maintained along with the repo under test.

Copy link
Contributor Author

@MatthiasBSchaefer MatthiasBSchaefer Mar 20, 2026

Choose a reason for hiding this comment

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

@beutlich
Thank you for your detailed review !

1.) I know, log is pretty verbose. But it is not really meant to be analyzed. Finding errors (like example Modelica.Blocks.Examples.xxx is not running anymore due to the changes in the PR) can be done in the published reports. The log is more or less to analyze possible errors in the workflow itself by us (LTX). We can reduce the logging once we are sure the workflow is stable. (without any changes inside this repository)

2.) probably we could introduce problem matchers, but may not be necessary due to 1.)

3.) We discussed this in the MAP-LIB meetings. Upload artifacts would induce the need to download the reports (which are not that small) , unzip and then open in a browser. These steps, we want to avoid, making the reports directly clickable by publishing on our server.

4.) Thank you for the very good hint with the variables instead of hard links!

5.)We can publish the main shell scripts called by the CI. But these are mainly running the Modelica tools inside docker containers. And we wont publish our regression testing tool itself, nor the docker files, due to intellectual property. So publishing the scripts is only partially helpful concerning transparency, but would induce a change of these scripts (and such a change in the MSL) in order to add a further Modelica tool (e.g. Simulation X) to the regression testing.

6.) Your proposed naming of the jobs is fine for me.


testrun_modelica:
needs: prepare
runs-on: self-hosted
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
runs-on: self-hosted
runs-on: [ self-hosted, linux, regression-testing ]

environment:
name: link_modelica
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
name: link_modelica
name: Build libraries

Not sure, what is meant by "link".

url: https://serv.ltx.de/prs/${{ github.event.number }}/Modelica/report/PR_comparison_report.html
Copy link
Member

@beutlich beutlich Mar 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Do not use hard-coded URIs for https://serv.ltx.de (here and in other places), instead prefer variables via https://github.com/modelica/ModelicaStandardLibrary/settings/variables/actions. This way LTX can move its server while workflow keeps untouched.

steps:
- name: run modelica
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
- name: run modelica
- name: Run tests

run: /home/resimuser/regression_testing/docker/work/run_scripts/run_pr.sh ${{ github.event.number }} ${{ github.event.pull_request.base.sha }} Modelica

testrun_modelicatest:
needs: prepare
runs-on: self-hosted
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
runs-on: self-hosted
runs-on: [ self-hosted, linux, regression-testing ]

environment:
name: link_modelicatest
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
name: link_modelicatest
name: Build libraries

Copy link
Contributor Author

@MatthiasBSchaefer MatthiasBSchaefer Mar 20, 2026

Choose a reason for hiding this comment

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

The enviroment is meant to create\show the links to the reports.
So "Report modelicatest" might be a good name

url: https://serv.ltx.de/prs/${{ github.event.number }}/ModelicaTest/report/PR_comparison_report.html
steps:
- name: run modelicatest
Copy link
Member

@beutlich beutlich Mar 19, 2026

Choose a reason for hiding this comment

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

Suggested change
- name: run modelicatest
- name: Run tests

run: /home/resimuser/regression_testing/docker/work/run_scripts/run_pr.sh ${{ github.event.number }} ${{ github.event.pull_request.base.sha }} ModelicaTest
Loading