Uh oh!
There was an error while loading. Please reload this page.
Added all single-file functionality to the lj script - #79
Conversation
…a.Applications error with a warning, and updated the README.
lam2mo
commented
Feb 11, 2020
@MrBartWolf Thanks! Will you also please change the |
lam2mo
commented
Feb 12, 2020
Sorry, that's not the problem. The problem is that you've removed the |
MrBartWolf
commented
Feb 12, 2020
The dotests.sh script will still fail since all of its scripts have been removed. I'm still working on rebuilding it, but it will likely take time. For now I will add the test.sh script for Travis. |
lam2mo
commented
Feb 13, 2020
Note: also update |
lam2mo
commented
Feb 14, 2020
Looks like |
lam2mo
commented
Feb 14, 2020
Excellent, thanks. @Zamua are you ok with merging all this in now? |
Actually, hang on. There seem to be some failures in the Travis logs even though for some reason it thinks it's passing. We should investigate that before we merge this in. EDIT FROM THE FUTURE: this is because the last remaining "failing" test is now being run by Travis. |
MrBartWolf
commented
Feb 18, 2020
The test that is failing is the primitive-numbers test that I'm fixing in another branch. As far as I can tell there is no other issue with the pull request. This request should be merged before the primitive-numbers one. |
Uh oh!
There was an error while loading. Please reload this page.
| @@ -124,14 +91,6 @@ if [ $# == 0 ]; then | |||
| exit | |||
There was a problem hiding this comment.
We should exit 1 if differences were detected. It is currently always exiting with 0 so travis will never fail
There was a problem hiding this comment.
It doesn't appear that the old dotests.sh script ever does exit 1 so I'm confused on what you mean.
There was a problem hiding this comment.
the old .travis.yml script didn't run the dotests.sh script. It ran ./compile.sh <file> && ./test.sh
If compile.sh failed then it would return a non zero exit code here: https://github.com/JMU-CS/less-java/blob/master/compile.sh#L50
If test.sh failed then it would return a non zero exit code here: https://github.com/JMU-CS/less-java/blob/master/test.sh#L17
Now that .travis.yml is not referencing these scripts and is instead referencing the dotest.sh script, the dotest.sh script must return non zero exit codes on failure to notify travis that testing has failed.
There was a problem hiding this comment.
Yes, we discussed this at the meeting today and @MrBartWolf has instructions for wrapping this up.
lam2mo
commented
Feb 21, 2020
@MrBartWolf Please provide an appropriate summary message on this thread once this is ready to be merged. Use a bullet list to describe each of the changes made. Note to whoever merges this PR once it's done: please squash with the aforementioned message. |
…a.Applications error with a warning, and updated the README.
…and eventually dotests.sh
…topped dotests.sh from testing files in the failing directory.
Summary of changes made by this pull request:
|
Zamua
commented
Feb 25, 2020
Looks good to me. All that's left is to rewrite the commit history a little to make it cleaner and easier to read. If you would like to do this yourself I recommend doing an interactive rebase. Below is an explanation of how to interactive rebase. Skip this if you are familiar with it. It will let you replay prior commits and will let you reorder, reword remove, edit, etc. I recommend rewording the top one, and "fixing up" (removing) the remaining. For the one you reword, I recommend pasting your entire summary comment into it. The git commands will look something like this: I also recommend checking out a new branch and experimenting first to ensure you understand what will happen and how to do it. If you'd rather not do this, then @lam2mo or I can do it ourselves before we merge in your PR. |
lam2mo
commented
Feb 26, 2020
@MrBartWolf was encountering some strange issues with the rebase so I decided just to squash and merge from the web interface to avoid prolonging this PR any more. Everything seems fine so far. |
Added testing of the program to the lj script, replaced the scary Java.Applications error with a warning, and updated the README.