unknownbrackets / ps2autotests

A set of test programs run on a PS2, with accompanying results.
ISC License
25 stars 7 forks source link

Updates to ee kernel thread stat test #6

Closed jpd002 closed 9 years ago

jpd002 commented 9 years ago

Various updates and fixes to thread stat test:

unknownbrackets commented 9 years ago

I'm thinking if this is a global problem, it makes sense to handle it globally in common.cpp (similar to how pspautotests does it, maybe.) There, a #define main test_main exists, and it forwards argc/argv on so it looks clean still.

Nice catch btw, I'm sure that was tricky to debug.

This would prevent this little reality from biting us with other tests. There may be other situations with the same problem.

In fact, we could extract out the BEGIN/END business there, which would be cleaner. I can do it if you'd like. I had the BEGIN/END in there for two reasons; first to filter out the additional logs from ps2link code, and second because sometimes ps2link provides truncated output and I wanted a systematic way to detect this.

Also, I like consistent code, but I'm more lax on specific tests as long as they're individually readable. For code used by multiple tests, though, I would like things lined up with spaces specifically so that it's easily readable in GitHub, Notepad, and IDEs concurrently (that is, tabs only for indentation.)

-[Unknown]

jpd002 commented 9 years ago

Sounds good, I'm going to move the test setup code in common.cpp and and add a #define main test_main in common.h.

Do you rebuild and recommit all test executables when doing a change like this one in the test framework?

unknownbrackets commented 9 years ago

Well, moving the BEGIN/END, probably would. If it's just adding optional features (like when I added the ability for checkpoint() to log timestamps), no, because it just clutters the git history size.

-[Unknown]

jpd002 commented 9 years ago

Question: In pspautotests, common is a .c file, which requires all .cpp test files to have their "main" function marked as extern "C" for proper linking. Do you want to stick to a common.c file for ps2autotests?

Also if we were to use a common file for IOP tests, we would probably need to have separate files for EE and IOP. Do you think it would be a good idea to rename the current common.c file to common-ee.cpp and common.h to common-ee.h?

unknownbrackets commented 9 years ago

Nah, it's just been a c file forever and we never changed it. I think cpp files are easier to deal with.

Sure, common-ee sounds fine.

-[Unknown]

jpd002 commented 9 years ago

Should I create a new pull request for the changes to common files? They actually touch a big portion of the files (because of the header file's new name) and they are not necessarily related to thread stat test improvements.

unknownbrackets commented 9 years ago

Either way. It does sound cleaner to start with a commit that only renames it and deals with that carnage first.

-[Unknown]