open-adventure merge requestshttps://staging.gitlab.com/esr/open-adventure/-/merge_requests2017-06-19T11:28:35Zhttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/106L12 take two2017-06-19T11:28:35Zusername-removed-815437L12 take twohttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/107Improved test coverage -- save/resume fail2017-06-19T17:28:57Zusername-removed-1096594aaron@traas.orgImproved test coverage -- save/resume failI've never done this kind of test-driven development, so it's been a good learning experience reading through the commit history. I figured I could lend a little bit of a hand in this case.I've never done this kind of test-driven development, so it's been a good learning experience reading through the commit history. I figured I could lend a little bit of a hand in this case.https://staging.gitlab.com/esr/open-adventure/-/merge_requests/112Master2017-06-19T17:55:47Zusername-removed-1096594aaron@traas.orgMasterTemporarily modified saveresume.c with version -1337, produced a save game file, reverted changes, and tested loading a file with the wrong version. This should give 100% coverage to saveresume.cTemporarily modified saveresume.c with version -1337, produced a save game file, reverted changes, and tested loading a file with the wrong version. This should give 100% coverage to saveresume.chttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/113Master2017-06-19T20:29:38Zusername-removed-1096594aaron@traas.orgMasterAdded additional coverage to actions.cAdded additional coverage to actions.chttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/114Further expanded test coverage on actions2017-06-20T14:27:54Zusername-removed-1096594aaron@traas.orgFurther expanded test coverage on actionshttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/117Test coverage - more corner cases in actions.c2017-06-20T17:40:35Zusername-removed-1096594aaron@traas.orgTest coverage - more corner cases in actions.chttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/118Don't exit on EOF from get_input().2017-06-20T17:44:34Zusername-removed-1340840Don't exit on EOF from get_input().https://staging.gitlab.com/esr/open-adventure/-/merge_requests/121Refactor turn threshold handling.2017-06-20T23:20:17Zusername-removed-1340840Refactor turn threshold handling.https://staging.gitlab.com/esr/open-adventure/-/merge_requests/122Respect CERT DCL16-C. - Use "L," not "l," to indicate a long value2017-06-21T17:06:11Zusername-removed-1362907Respect CERT DCL16-C. - Use "L," not "l," to indicate a long valuehttps://sonarcloud.io/organizations/open-adventure/rules#rule_key=c%3ALiteralSuffix
- MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
- MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
...https://sonarcloud.io/organizations/open-adventure/rules#rule_key=c%3ALiteralSuffix
- MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
- MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
- CERT DCL16-C. - Use "L," not "l," to indicate a long value
- CERT DCL16-CPP. - Use "L," not "l," to indicate a long value
- CERT, DCL50-J. - Use visually distinct identifiershttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/126Replace SETPRM/[PR]SPEAK with variadic [pr]speak2017-06-21T17:13:09Zusername-removed-815437Replace SETPRM/[PR]SPEAK with variadic [pr]speakRename/rewrite old speak to vspeak and take a va_list
Add new speak that takes variadic parameters
Remove SETPRM & PARMS[]Rename/rewrite old speak to vspeak and take a va_list
Add new speak that takes variadic parameters
Remove SETPRM & PARMS[]https://staging.gitlab.com/esr/open-adventure/-/merge_requests/124Respect CERT, DCL04-C. - Do not declare more than one variable per declaration2017-06-21T19:21:47Zusername-removed-1362907Respect CERT, DCL04-C. - Do not declare more than one variable per declarationhttps://sonarcloud.io/organizations/open-adventure/rules#rule_key=c%3ASingleDeclarationPerStatement
- MISRA C++:2008, 8-0-1 - An init-declarator-list or a member-declarator-list shall consist of a single init-declarator or member-declar...https://sonarcloud.io/organizations/open-adventure/rules#rule_key=c%3ASingleDeclarationPerStatement
- MISRA C++:2008, 8-0-1 - An init-declarator-list or a member-declarator-list shall consist of a single init-declarator or member-declarator respectively
- CERT, DCL52-J. - Do not declare more than one variable per declaration
- CERT, DCL04-C. - Do not declare more than one variable per declaration
- CERT, DCL04-CPP. - Do not declare more than one variable per declarationhttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/128WIP: Respect c:S1871 Two branches in a conditional structure should not have ...2017-06-21T21:44:30Zusername-removed-1362907WIP: Respect c:S1871 Two branches in a conditional structure should not have exactly the same implementationI am of two minds about this one and have marked it as work in progress.
Nine of the state transitions have the same implementation (`RSPEAK` then `GO_CLEAROBJ`) and are flagged as duplicate code. This duplication is a classic maintenan...I am of two minds about this one and have marked it as work in progress.
Nine of the state transitions have the same implementation (`RSPEAK` then `GO_CLEAROBJ`) and are flagged as duplicate code. This duplication is a classic maintenance problem. If these actions are *intrinsically* equivalent, then the code should be factored. (If they are *coincidentally* equivalent, then they shouldn't be factored.)
The obvious way to factor the code is to group the cases, as is suggested in this MR. However, this breaks the comforting treatment of the cases in numerical order, which makes it easy to check that all of the cases are handled, and “reads” like a table.
I think that the factoring proposed by this MR preserves the algorithm of the original, because the same state transitions are defined. But I understand that some might feel that defining them with a table was part of the original intent, and must be preserved.
https://sonarcloud.io/organizations/open-adventure/rules#rule_key=c%3AS1871
- c:S1871 Two branches in a conditional structure should not have exactly the same implementationhttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/131Fixups to the adventure.yaml commentary.2017-06-22T18:38:16Zusername-removed-1340840Fixups to the adventure.yaml commentary.https://staging.gitlab.com/esr/open-adventure/-/merge_requests/123Respect MISRA C:2004, 9.3 - In an enumerator list, the "=" construct shall ...2017-06-26T19:10:13Zusername-removed-1362907Respect MISRA C:2004, 9.3 - In an enumerator list, the "=" construct shall ...https://sonarcloud.io/organizations/open-adventure/rules#rule_key=c%3AEnumPartialInitialization
- MISRA C:2004, 9.3 - In an enumerator list, the "=" construct shall not be used to explicitly initialize members other than the first, unle...https://sonarcloud.io/organizations/open-adventure/rules#rule_key=c%3AEnumPartialInitialization
- MISRA C:2004, 9.3 - In an enumerator list, the "=" construct shall not be used to explicitly initialize members other than the first, unless all items are explicitly initialized.https://staging.gitlab.com/esr/open-adventure/-/merge_requests/132Created cheat save file generator for testing purposes.2017-06-26T19:13:37Zusername-removed-1096594aaron@traas.orgCreated cheat save file generator for testing purposes.What I was trying to do before was fragile; I committed a save file that was hacked. However, if the game_t or save_t ever changed, that could break the test.
So I created a new make target: cheat, which creates a binary called cheat. C...What I was trying to do before was fragile; I committed a save file that was hacked. However, if the game_t or save_t ever changed, that could break the test.
So I created a new make target: cheat, which creates a binary called cheat. Cheat uses the advent structures and functions to generate a legitimate save file with numdie = -1000. Then we use that to run tests.
I had to modify suspend() to seperate out as savefile() the part that generates the save file from the part that asks for user input, so I could call that directly from the main() in cheat. I also made a new make target, and execute the cheat binary in tests/Makefile
If this is not the approach to achieve coverage that you want to see, let me know.https://staging.gitlab.com/esr/open-adventure/-/merge_requests/140Replace motion word VOCWRD() calls with enums.2017-06-27T02:23:36Zusername-removed-1340840Replace motion word VOCWRD() calls with enums.To do this, I had to reexpress motion words themselves in adventure.yaml.To do this, I had to reexpress motion words themselves in adventure.yaml.https://staging.gitlab.com/esr/open-adventure/-/merge_requests/143cheat now has command-line arguments2017-06-27T21:00:22Zusername-removed-1096594aaron@traas.orgcheat now has command-line arguments
-d number of deaths. Signed integer value
-s number of saves. Signed integer value
-s version number of save file. Signed integer value
-o file name of save game to write
I've also eliminated the pre-generated save file I u...
-d number of deaths. Signed integer value
-s number of saves. Signed integer value
-s version number of save file. Signed integer value
-o file name of save game to write
I've also eliminated the pre-generated save file I used to do coverage of the version mismatch error, and used this to generate.
Also, got 100% test coverage of cheat.chttps://staging.gitlab.com/esr/open-adventure/-/merge_requests/144Test coverage improvements and cleanup2017-06-28T14:58:36Zusername-removed-1096594aaron@traas.orgTest coverage improvements and cleanupCleaner `make check` output -- no longer showing error messages to console for tests. Also, improved coverage.Cleaner `make check` output -- no longer showing error messages to console for tests. Also, improved coverage.https://staging.gitlab.com/esr/open-adventure/-/merge_requests/146Make LCOV ignore code unreachable in tests2017-06-28T17:27:08Zusername-removed-1096594aaron@traas.orgMake LCOV ignore code unreachable in testsI'm not sure if you want to use this tactic or not -- this is my first time working on a project with coverage testing, so I'm unsure of the wisdom of what I've done.
I've identified 3 types of code that I believe to be unreachable in t...I'm not sure if you want to use this tactic or not -- this is my first time working on a project with coverage testing, so I'm unsure of the wisdom of what I've done.
I've identified 3 types of code that I believe to be unreachable in testing:
1) code that requires interactivity (like being able to type ^C)
2) code that tests for an OOM error
3) bugs, that should be unreachable by design
I've made 3 different commits, ordered in how certain I am about their legitimacy:
9a4dccf4 - sig_handler() and OOM check in xmalloc()
94bf6404 - bug() and all calls to BUG()
d04e1e58 - some bits of misc.c that I *think* are unreachable because they require interactivity, but could be wrong
Also: am I correct that dungeon.c is going away eventually, and thus I shouldn't worry about getting perfect test coverage there?https://staging.gitlab.com/esr/open-adventure/-/merge_requests/147Limited cleanup2017-06-28T17:43:29Zusername-removed-1354299Limited cleanup