BN test fails sporadically

[mr-tz]

see for example https://github.com/mandiant/capa/actions/runs/4883990091/jobs/8716121459

FAILED tests/test_binja_features.py::test_binja_features***mimikatz-function=0x4556E5-characteristic(stack string)-True*** - AssertionError: characteristic(stack string) should be found in function=0x4556E5

[williballenthin]

seems that the BN stack strings extraction is flakey. maybe there's a race or some other non-determinism.

@xusheng6 thoughts?

[williballenthin]

also here: https://github.com/mandiant/capa/actions/runs/4996267982/jobs/8949779703

[xusheng6]

We have made some improvements to the stack string detection. The old code is unaware of it and fails. I will update the code and fix this

[williballenthin]

ahh, thats right, there's been a BN release. im used to our deps being pinned, so without action, tests usually don't start failing. i will add an additional BN test that asserts the current expected version and can highlight when an update is released and may break tests.

[xusheng6]

It should not be too hard to leverage the new stack string detection in BN and fix the test for capa. However, I have some work at hand and cannot work on it immediately.

On the other hand, I know it could be quite annoying to block your automated tests. Luckily, there is a workaround for it: disable the analysis.outlining.builtins setting, which will disable the new stack string detection mechanism. And the behavior should go back to its previous way and the unit test should pass.

Here is an example on how to disable (and then turn back on) a BN setting: https://github.com/mandiant/capa/blob/master/tests/fixtures.py#L169-L176. So @williballenthin if you think it is urgent to fix the failure and unblock the CI, you can add a few lines of code to disable the setting (there is even no need to turn it back on). Otherwise, if you are fine with waiting for me for a few days, I can fix the issue within the week

[williballenthin]

It should not be too hard to leverage the new stack string detection in BN and fix the test for capa. However, I have some work at hand and cannot work on it immediately.

no worries, we'll work on this shortly!

[xusheng6]

I am getting back to this and will check out the current status of it

[mr-tz]

this is still occurring, see e.g. https://github.com/mandiant/capa/actions/runs/5258849447/jobs/9503659030

get_extractor = <functools._lru_cache_wrapper object at 0x7fe8d97ec720>
sample = '/home/runner/work/capa/capa/tests/data/mimikatz.exe_'
scope = <function resolve_scope.<locals>.inner_function at 0x7fe8bcdca160>
feature = characteristic(stack string), expected = True

    def do_test_feature_presence(get_extractor, sample, scope, feature, expected):
        extractor = get_extractor(sample)
        features = scope(extractor)
        if expected:
            msg = f"***str(feature)*** should be found in ***scope.__name__***"
        else:
            msg = f"***str(feature)*** should not be found in ***scope.__name__***"
>       assert feature.evaluate(features) == expected, msg
E       AssertionError: characteristic(stack string) should be found in function=0x4556E5

tests/fixtures.py:1059: AssertionError
=============================== warnings summary ===============================
tests/test_binja_features.py::test_standalone_binja_backend
  /opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/halo/halo.py:497: DeprecationWarning: setDaemon() is deprecated, set the daemon attribute instead
    self._spinner_thread.setDaemon(True)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
FAILED tests/test_binja_features.py::test_binja_features***mimikatz-function=0x4556E5-characteristic(stack string)-True*** - AssertionError: characteristic(stack string) should be found in function=0x4556E5
============= 1 failed, 158 passed, 1 warning in 850.60s (0:14:10) =============
##***error***Process completed with exit code 1.

[xusheng6]

This is weird, I will have a look at it later.

[xusheng6]

I cannot reproduce this locally. Though I am testing on a macOS now. I will grab the headless build and try it on my Linux box

[mr-tz]

Should we xfail/disable this binary ninja test for now?

[xusheng6]

Should we xfail/disable this binary ninja test for now?

Sorry I have not taken the time to look at this. Been busy with other tasks. Sorry for the inconvenience. I know it does not feel good if the CI always fails.

I have no objection on disabling it for now. I will set up my own CI on GitHub and see why is actually going on.

[mr-tz]

No worries, we know you're busy. I'll disable this for now.

[xusheng6]

@williballenthin I think this issue can be closed since we have merged #1669

[williballenthin]

thanks!