Skip to content

Conversation

@tijcolem
Copy link
Collaborator

@tijcolem tijcolem commented Oct 29, 2021

Pull request overview

This adds in a new GitHub actions to run regressions tests on all platforms using both cli and ruby bindings. I had to create a fork in order to test the GitHub actions as it needs to be on develop (at least I think it does). It's setup pretty much the same as the Test SDK Installer except the branch commits. I suppose this could be added but I think we can use the Test SDK Installer workflow to commit the test results.

This one is still running as of now. https://github.com/tijcolem/OpenStudio-resources/actions/runs/1397133553
Just waiting to make sure all steps work but I think this is close.

@tijcolem tijcolem added the Other PR type: Something else, like maintenance of the repo, or just committing test results label Oct 29, 2021
@tijcolem tijcolem requested a review from jmarrec October 29, 2021 13:09
Copy link
Contributor

@jmarrec jmarrec left a comment

Choose a reason for hiding this comment

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

Couple of small adjustments needed but it's 95% there! Nice!

@@ -0,0 +1,41 @@
function Controller () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as the mac one really you need a single install-script.qs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair point. this is done

os_installer_ubuntu_1804:
description: 'Ubuntu 1804 (.deb) URL'
required: true
default: 'https://github.com/NREL/OpenStudio/releases/download/v3.2.1/OpenStudio-3.2.1%2Bbdbdbc9da6-Ubuntu-18.04.deb'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is tedious to launch. We can infer the installer links for all platforms from a single one supplied.

Copy link
Collaborator Author

@tijcolem tijcolem Nov 2, 2021

Choose a reason for hiding this comment

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

I suppose OS builds will always have the base url.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well I take that back. Windows is a little odd as I push out both unsigned and signed installers so s3 on nightly build that we'll certainly want to test and hook into this action.

e.g. http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/?prefix=3.3.0-rc2/
signed: http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/?prefix=3.3.0-rc2/signed/

Even though it's a bit tedious to enter 4 links there may be cases it's needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

So you went back on this idea of having 4 links?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's now just base url along with build sha, version.

ruby -v
MT_CPU=$(nproc) ruby model_tests.rb
- name: Generate HTML and heatmap
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably mean to run this step twice, after each run of model_tests.rb (cli + run)

You probably want to run with CUSTOMTAG=cli and CUSTOMTAG=ruby

And call process_results.py with --tagged or --all

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, that's a good idea to tag them as cli and ruby

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to run model_tests.rb with both openstudio (CLI) and system ruby? That's gonna take a long time. The fact that we would want to do it on Windows I can see, but linux/mac I'm not sure.

Copy link
Collaborator Author

@tijcolem tijcolem Apr 13, 2022

Choose a reason for hiding this comment

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

I plan on these running nightly so I don't mind if they take a while to run. If we want to hook this into PRs on OpenStudio I think we should just run cli and maybe just one platform but we can use the other workflow for that. Maybe rename the workflow to _installer or something along those lines.


jobs:
installer_ubuntu_1804:
runs-on: ubuntu-18.04
Copy link
Contributor

Choose a reason for hiding this comment

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

There's code that's duplicated for each job, but using a matrix and a bunch of if platform (in bash or conditionally run some steps) is probably not going to simplify much so I'm fine with it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I thought of this. My understanding is the matrix works great for testing things such as multiples versions of python, not sure how if matrix works for vm instances.

run: |
set -x
echo "Installer link: ${{ github.event.inputs.os_installer_ubuntu_1804 }}"
installer_url="${{ github.event.inputs.base_url}}OpenStudio-${{ github.event.inputs.os_version}}${{ github.event.inputs.os_prerelease_tag}}%2B${{ github.event.inputs.os_build_sha}}-Ubuntu-18.04.deb"
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a bit fragile, if you forgot the trailing slash in base_url?

run: |
set -x
echo "Installer link: ${{ github.event.inputs.os_installer_ubuntu_1804 }}"
installer_url="${{ github.event.inputs.base_url}}OpenStudio-${{ github.event.inputs.os_version}}${{ github.event.inputs.os_prerelease_tag}}%2B${{ github.event.inputs.os_build_sha}}-Ubuntu-18.04.deb"
Copy link
Contributor

@jmarrec jmarrec Apr 13, 2022

Choose a reason for hiding this comment

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

Same would need to be applied to all jobs

Suggested change
installer_url="${{ github.event.inputs.base_url}}OpenStudio-${{ github.event.inputs.os_version}}${{ github.event.inputs.os_prerelease_tag}}%2B${{ github.event.inputs.os_build_sha}}-Ubuntu-18.04.deb"
base_url="${{ github.event.inputs.base_url}}"
# remove trailing slash if any
base_url=${base_url%/}
installer_url="$base_url/OpenStudio-${{ github.event.inputs.os_version}}${{ github.event.inputs.os_prerelease_tag}}%2B${{ github.event.inputs.os_build_sha}}-Ubuntu-18.04.deb"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, this would be good to add.

os_installer_ubuntu_1804:
description: 'Ubuntu 1804 (.deb) URL'
base_url:
description: 'The base url to use to download each platform'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect to use builds from s3 instead of a prerelease? just curious

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I recently used s3 as a test and works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Other PR type: Something else, like maintenance of the repo, or just committing test results

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants