Test for unification of path filtering across POSIX and Windows

[pcaversaccio]

As discussed with @0xalpharush in #1196 I now wrote a small test suite that checks if path filtering across POSIX and Windows works.

The CI tests will fail with the error "Path filtering across POSIX and Windows failed" because you use the master branch in the CI to set up Slither. So this is a good error :-D. I tested it here.

The strategy I took here is that I made a couple of contracts that contain reentrancy issues as dependencies to a main (empty) contract. Now if you filter out the paths properly, the output should be zero result(s) found.

I hope the code is in line with your internal rules (not sure about the solc version for the test contracts).

[CLAassistant]

All committers have signed the CLA.

[pcaversaccio]

@montyly I just saw you re-triggered the actions. Is there something wrong with the code?

As mentioned above, the reason why it's failing (and it's actually good) is because of how you set up the CI flow.

You use python setup.py install and in the setup.py you use Slither version 0.8.3 which does not contain the fix from #1196.

[montyly]

@pcaversaccio: my bad; there were some timeout issues in the CI due to too many urls requests, so I just went ahead an re-run the recent PRs with failing actions to see if they were affected :)

[pcaversaccio]

@montyly lol - had a similar problem recently.

[pcaversaccio]

@0xalpharush @montyly any feedback on this PR? IMHO it's ready to be merged.

[pcaversaccio]

hey guys, @0xalpharush @montyly - it's been already some time since I prepared that PR. Would appreciate some feedback or let's get it merged if you agree, thx!

[elopez]

Thanks for the PR @pcaversaccio & apologies for the delayed review! I've left some comments inline, but overall this looks ok. The only thing that needs to be addressed is the change due to the introduction of the new --fail-* options in slither. If you could merge dev into your branch to get the new code in and address the comments in the PR that'd be awesome!

[pcaversaccio]

@elopez thanks for the feedback - dev is merged into my branch via https://github.com/pcaversaccio/slither/pull/1. I searched for your inline comments but couldn't find anything - could you please point me to your feedback, thx.

[elopez]

For some reason GitHub didn't submit them together with my comment earlier, my apologies. You should see them now.

[elopez]

It looks like the CI test fails @pcaversaccio 🤔 could you take a look? I suspect it might need solc-select install & use 0.8.0 in the script (the pipeline runs with a 0.5.x version otherwise)

[pcaversaccio]

@elopez I already indicated here why the tests are failing. Since we test against Slither version 0.8.3 which does not contain your fix the test is failing (which is actually good since it does its job). Don't know how to resolve this tbh.

[elopez]

@pcaversaccio the tests install slither from dev (or whatever the PR branch is built off of) source code, which already has the changes in #1196 merged in, so the tests should be passing. The version number is not relevant for this, as the release is not pulled from PyPI. I think it's just a matter of the test solidity code failing to build, as the CI pipeline uses solc 0.5.x and the code requires 0.8.x

[pcaversaccio]

@elopez you are completely right! I pushed a fix - could re-trigger the actions?

[pcaversaccio]

yeah :-D everything green. thx for the hint!

[montyly]

This is great, thanks @pcaversaccio