search

Dexels / navajo

Navajo Service-oriented Applications
GNU Affero General Public License v3.0
9 stars 5 forks source link

Fix n586 recompile depencies on delete #597

Closed kharybdys closed 2 years ago

kharybdys commented 3 years ago

Includes adding support for testing this

kharybdys commented 3 years ago

I'm planning on adding more tests in the near future

ghost commented 2 years ago

Your unit tests print text, which I think is considered "bad form", they should simply either succeed silently or fail with a clear stack trace. Any other text is distracting.

kharybdys commented 2 years ago

Your unit tests print text, which I think is considered "bad form", they should simply either succeed silently or fail with a clear stack trace. Any other text is distracting.

Are you bothered with the System.out.println's or with the single assertTrue in the helper function deleteFilesAndcreateDeleteEventForFiles ? The first are there purely for debugging purposes, the latter is because a) it's a helper function and b) it shouldn't happen, but it's important to catch that exception there I think. For this one you'd just not catch the exception?

ghost commented 2 years ago

I am refering to the System.*.println statements. If they are there to debug the unit tests themselves, they should be removed before a pull request (because they are no longer needed). If they are there to debug the actual code itself, I am confused, since the unit test runners give the same kind of information.

kharybdys commented 2 years ago

I found them useful to figure out which part of my console was about the test that just failed when running the full suite (and yes I know I can rerun the failing test on its own to get the same effect). I would've put them on logger.debug level except that, as far as I know, JUnit doesn't have a logger implementation. But I'll remove them, my JUnit skills probably need some improving.