Skip to content

Automatic compilation checking via github actions#64

Merged
MarcCote merged 11 commits intoallenai:mainfrom
AndKaminer:main
Dec 22, 2023
Merged

Automatic compilation checking via github actions#64
MarcCote merged 11 commits intoallenai:mainfrom
AndKaminer:main

Conversation

@AndKaminer
Copy link
Contributor

@AndKaminer AndKaminer commented Dec 20, 2023

Use this github action to setup scala. Then run package.sh. If it fails, the test fails and vice versa. The only problem with this is that it behaves oddly on Windows. The action can only pass on windows. It never fails, even when it fails on unix-based systems.

Run ./simulator/package.sh
  ./simulator/package.sh
  shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"
  env:
    pythonLocation: C:\hostedtoolcache\windows\Python\3.11.7\x64
    PKG_CONFIG_PATH: C:\hostedtoolcache\windows\Python\3.11.7\x64/lib/pkgconfig
    Python_ROOT_DIR: C:\hostedtoolcache\windows\Python\3.11.7\x64
    Python2_ROOT_DIR: C:\hostedtoolcache\windows\Python\3.11.7\x64
    Python3_ROOT_DIR: C:\hostedtoolcache\windows\Python\3.11.7\x64
    CI: true
    JAVA_HOME: C:\Users\runneradmin\.jabba\jdk\[email protected]

This ^ is the output of the test on Windows.


Run ./simulator/package.sh
~/work/ScienceWorld/ScienceWorld/simulator ~/work/ScienceWorld/ScienceWorld
Downloading sbt launcher for 1.9.7:
  From  https://repo1.maven.org/maven2/org/scala-sbt/sbt-launch/1.9.7/sbt-launch-1.9.7.jar
    To  /home/runner/.sbt/launchers/1.9.7/sbt-launch.jar
Downloading sbt launcher 1.9.7 md5 hash:
  From  https://repo1.maven.org/maven2/org/scala-sbt/sbt-launch/1.9.7/sbt-launch-1.9.7.jar.md5
    To  /home/runner/.sbt/launchers/1.9.7/sbt-launch.jar.md5
/home/runner/.sbt/launchers/1.9.7/sbt-launch.jar: OK
[info] [launcher] getting org.scala-sbt sbt 1.9.7  (this may take some time)...
[info] [launcher] getting Scala 2.12.18 (for sbt)...
[info] Updated file /home/runner/work/ScienceWorld/ScienceWorld/simulator/project/build.properties: set sbt.version to 1.9.7
[info] welcome to sbt 1.9.7 (AdoptOpenJDK Java 1.8.0_292)
[info] loading settings for project simulator-build from plugins.sbt ...
[info] loading project definition from /home/runner/work/ScienceWorld/ScienceWorld/simulator/project
[info] loading settings for project simulator from build.sbt ...
[info] set current project to scienceworld-scala (in build file:/home/runner/work/ScienceWorld/ScienceWorld/simulator/)
[info] compiling 182 Scala sources to /home/runner/work/ScienceWorld/ScienceWorld/simulator/target/scala-2.12/classes ...
[info] Non-compiled module 'compiler-bridge_2.12' for Scala 2.12.9. Compiling...
[info]   Compilation completed in 7.929s.
[info] done compiling
[info] Strategy 'discard' was applied to 3 files (Run the task at debug level to see details)
[info] Strategy 'rename' was applied to 2 files (Run the task at debug level to see details)
[success] Total time: 26 s, completed Dec 20, 2023 2:21:08 PM
~/work/ScienceWorld/ScienceWorld

Whereas this ^ is the output on unix systems.

It appears to me that the script isn't even running on windows. Unfortunately, I don't have a windows machine, so I can't directly test on windows.

Regardless, if something goes wrong with the compilation, tests do still fail. You just only see it in macos or ubuntu.

MarcCote
MarcCote requested changes Dec 20, 2023
View reviewed changes
Copy link
Collaborator

@MarcCote MarcCote left a comment

Choose a reason for hiding this comment

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

We could avoid compiling the jar on all individual runs by making the test runs depend on a parent run that compiles the jar. That parent run could run on Linux.

.github/workflows/tests.yaml Outdated
Comment on lines +33 to +35
- uses: olafurpg/setup-scala@v10
- name: Check scienceworld compilation
run: ./simulator/package.sh
Copy link
Collaborator

@MarcCote MarcCote Dec 20, 2023

Choose a reason for hiding this comment

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

Ideally, we want to use that compiled jar file to run the tests as well.

Copy link
Collaborator

@MarcCote MarcCote Dec 20, 2023

Choose a reason for hiding this comment

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

Artifact can be share between runs https://docs.github.com/en/actions/using-workflows/storing-workflow-data-as-artifacts

@MarcCote
Copy link
Collaborator

MarcCote commented Dec 21, 2023

@AndKaminer I made PR to your branch with the changes I have in mind.
AndKaminer#1

This is how it looks like when it is running
https://github.com/allenai/ScienceWorld/actions/runs/7289142662

@AndKaminer
Copy link
Contributor Author

AndKaminer commented Dec 22, 2023

@AndKaminer I made PR to your branch with the changes I have in mind. AndKaminer#1

This is how it looks like when it is running https://github.com/allenai/ScienceWorld/actions/runs/7289142662

Thanks for doing that. Sorry for taking out a PR and ghosting. I had some unexpected things come up. I'll take a look.

@AndKaminer
Merge pull request #1 from allenai/AndKaminer-main
3b532e0 
Use a separate Github Action to build the JAR.
@AndKaminer
Copy link
Contributor Author

AndKaminer commented Dec 22, 2023

@AndKaminer I made PR to your branch with the changes I have in mind. AndKaminer#1
This is how it looks like when it is running https://github.com/allenai/ScienceWorld/actions/runs/7289142662

Thanks for doing that. Sorry for taking out a PR and ghosting. I had some unexpected things come up. I'll take a look.

Looks great! I just merged on my branch, so it should be good to merge here.

@MarcCote
Copy link
Collaborator

MarcCote commented Dec 22, 2023

@AndKaminer I made PR to your branch with the changes I have in mind. AndKaminer#1
This is how it looks like when it is running https://github.com/allenai/ScienceWorld/actions/runs/7289142662

Thanks for doing that. Sorry for taking out a PR and ghosting. I had some unexpected things come up. I'll take a look.

No worries. Thanks for initiating all the changes.

@MarcCote
Copy link
Collaborator

MarcCote commented Dec 22, 2023

Fixes #60

@MarcCote MarcCote merged commit 834f33a into allenai:main Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarcCote MarcCote MarcCote requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AndKaminer @MarcCote