ethereum / hevm

symbolic EVM evaluator
https://hevm.dev
GNU Affero General Public License v3.0
225 stars 46 forks source link

Fix `--root` option for `test` subcommand #449

Closed msooseth closed 7 months ago

msooseth commented 7 months ago

Description

The --root option for the test subcommand didn't work actually. This is because we cwd-d into the directory, then called all the functions with the directory name as a prefix, so the directory was in a sense prefixed twice. This is easy to test -- create a tmp subdirectory and cd into it, call git init, call forge init --force, add a solidity file under src/ run forge build then go back with cd .. and then run hevm test --root "tmp/" it will fail.

With this change, it now succeeds, as expected.

Checklist

arcz commented 7 months ago

I think this is good. Also, getCurrentDirectory returns absolute path but --root can be relative so it might be better to use makeAbsolute in getRoot so it always returns absolute path:

getRoot cmd = maybe getCurrentDirectory makeAbsolute (cmd.root)
msooseth commented 7 months ago

I think this is good. Also, getCurrentDirectory returns absolute path but --root can be relative so it might be better to use makeAbsolute in getRoot so it always returns absolute path:

getRoot cmd = maybe getCurrentDirectory makeAbsolute (cmd.root)

That's a really good point, thanks! I was actually confused when I was debugging what that relative path was that it was referring to. It would have been a lot easier to understand the bug with this already there. I added it in. I'll wait for the checks to finish and then merge it in :)