Negative unit test for bad patches

[JackMaguire]

I'm making a unit test out of my recent post in the debugging channel. It may not be the perfect error message but it is better than a seg fault and invites the user to expand upon the error message.

I wrote this code in the browser so I'm expecting a bit of beautification and typo correction to be necessary.

[JackMaguire]

It looks like the test ran and passed but didn't output any log. Maybe that is expected for unit tests?

[JackMaguire]

How does beautification work now with the fork system?

[roccomoretti]

I might recommend TS_ASSERT_THROWS_ANYTHING() (and/or TS_ASSERT_THROWS_NOTHING()) -- see test/core/scoring/ScoreFunctionFactory.cxxtest.hh for example usage.

[roccomoretti]

How does beautification work now with the fork system?

I think that if "Maintainers are allowed to edit this pull request." is checked, it should be possible to manually queue the "beautify" test on the test server, and have the test submit the update to the PR ... that said, I'm not sure if we have all the settings lined up to have that work in the new system. (I've submitted a test case and we'll see if it works.)

Failing that, you can always run things locally & manually with python ../tools/python_cc_reader/beautify_changed_files_in_branch.py

[lyskov]

It looks like the test ran and passed but didn't output any log. Maybe that is expected for unit tests?

@JackMaguire fyi: passed unit test logs are cleared by default

[lyskov]

How does beautification work now with the fork system?

Also, our old "beautify" button should still work (i think 🤔)…

[JackMaguire]

Hmm I don't actually have a computer that I can use for beautification. The testing server seemed to get confused:

Could not figure out which branch to beautify, commit cb021a9889835d9ffedd1969deaf16c1cf1ef314
 belong to following branches (expecting exactly one origin/<branch> or origin/main to be present):
* (HEAD detached from ba06f11430)

Aborting...

[lyskov]

Hm… looks like out current restriction actually prevent beautification procedure to commit results (see logs here https://b3.graylab.jhu.edu/test/842562). I added this as topic for the next devel meting to see how it will be best to workaround this issue.

[roccomoretti]

@lyskov That's for beautification of the main branch -- I believe Jack is looking for the slightly different use case of beautifying a (as yet not merged) PR branch, where the branch is in a repo fork. (So https://b3.graylab.jhu.edu/test/842257)

[lyskov]

Ah, i see, - yeah, our old approach will not work with forked-PR, hm… 🤔

[JackMaguire]

Sorry to be difficult about this. It is just that my only personal computer is a macbook air and I don't have developer tools installed, so I can't run any of this locally.

Would it be a lot of work to have the testing server spit out a git diff whenever it doesn't know where to push the commit? I could just beautify by hand.

[roccomoretti]

@JackMaguire PR #204 is merged now. If you can merge main into this branch, the test server should be able to give you the beautification diffs.

[JackMaguire]

Thanks for adding that, @roccomoretti ! The beautification tests pass, the new unit test passes, and I updated the code to use TS_ASSERT_THROWS_ANYTHING. Does this look good to merge?