Fix: use if/else pattern for skip() guards in unit tests#1
Closed
will-bates11 wants to merge 1 commit intomainfrom
Closed
Fix: use if/else pattern for skip() guards in unit tests#1will-bates11 wants to merge 1 commit intomainfrom
will-bates11 wants to merge 1 commit intomainfrom
Conversation
Nim's skip() template marks a test as SKIPPED but does not stop
execution -- the test body continues running. Tests that called
skip() and then used check() after it were being marked FAILED
rather than SKIPPED when optional native libraries (Capstone,
Keystone, Unicorn) were absent.
Fixed by wrapping the test body in an else branch so checks only
execute when the library is actually available:
test_assembler.nim - Keystone-dependent tests (6 tests)
test_disassembler.nim - Capstone-dependent tests (6 tests), also
removed the now-redundant requireCapstone
helper template
test_emulator.nim - Unicorn-dependent tests (8 tests)
test_patcher.nim - Keystone/Unicorn-dependent tests (5 tests)
test_runtime.nim - Qualified ambiguous calls (attachProcess,
detachProcess, etc.) with runtime. prefix
to fix compile error when process module
is also imported
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skip()template marks tests as SKIPPED but does not halt test execution. The test body continues running afterskip()is called.if not isLibAvailable(): skip()followed bycheckassertions were being marked FAILED (not SKIPPED) when optional native libraries (Capstone, Keystone, Unicorn) were absent, because the checks ran regardless.else:so they only execute when the library is present.test_runtime.nimwhere bothruntimeandprocessexportattachProcessand related procs -- qualified all calls withruntime.prefix.Files changed
tests/test_assembler.nim- 6 Keystone-dependent teststests/test_disassembler.nim- 6 Capstone-dependent tests, removed unusedrequireCapstonetemplatetests/test_emulator.nim- 8 Unicorn-dependent teststests/test_patcher.nim- 5 Keystone/Unicorn-dependent teststests/test_runtime.nim- compile error fix: qualify ambiguous proc names withruntime.module prefixTest plan
nim c -d:release --path:src -o:nimguard.exe src/nimguard.nimtest_runtime.nimcompiles cleanly without ambiguous call error