Seeing bad cache invalidation in makerdao/spells-mainnet

[hexonaut]

Steps to reproduce:

1. Fork https://github.com/makerdao/spells-mainnet and checkout commit be8c6e946a5fa7768d8af0f6e200712f397ad587 .
2. Remove the --force arg from test-dssspell-forge.sh.
3. Run make test-forge.
4. Tests should correctly mostly error (4 success, 9 failing) and report that the spell has already been cast. Cache has been built.
5. Edit src/DssSpell.t.base.sol line 294 and set the deployed_spell to address(0).
6. Run make test-forge.

Expected output:

Most tests should start passing (11 succeed, 2 failing) with no mention of spell already cast.

Actual output:

Identical from the first test which is incorrect.

[wilsoncusack]

I think same issue as #299. cc @mattsse who had asked for a repo to reproduce

[mattsse]

thanks for this,
this is also currently tracked here gakonst/ethers-rs#739 as this is an ethers-solc issue, which should be resolved shortly.

[mattsse]

oh I finally figured out what's causing this,
the change is correctly detected, however we're failing to recompile the DssSpellTest which inherits DssSpellTestBase. so we're recompiling DssSpellTestBase but not DssSpellTest

I believe this is simply an oversight in the change detection,
working on a fix.

[hexonaut]

Tried this again. Still getting the same issue. Oddly enough I'm using test inheritance in another project and that one works fine.

Here is steps to reproduce on spells-mainnet:

1. Load this branch: https://github.com/makerdao/spells-mainnet/tree/add-forge-test and run forge test --fork-url "$ETH_RPC_URL" (set the rpc url)
2. Change any line in DssSpell.t.base.sol. In particular I swap L257 to use either the address(0) or address(1).

You should see a change in the number of failing tests 4 succeeding vs 11.

Here is another repo where I've used test inheritance: https://github.com/makerdao/dss-test/blob/master/src/tests/IntegrationTest_0_8_9.t.sol

This one seems to work without --force for some reason. Hope this helps.

[mattsse]

thanks!
very helpful,

tried to replicate the case here

https://github.com/gakonst/ethers-rs/blob/4d2cd83698788ae090aeefd508c27631bd8d5f12/ethers-solc/tests/project.rs#L141-L222

which seems to work properly,
but this will help me to track down the root issue

[hexonaut]

Have you tried my exact replication steps outside of that test? There is something in particular about spells-mainnnet that is broken.

[mattsse]

finally found what I believe is the actual root cause for this, it has to do with how we're currently managing the absolute path and the source name of contracts which gets mixed up somehow, probably a remappings/cache/absolute path snafu.

this is also the cause for gakonst/ethers-rs#727.

I know how to fix that now and also test against it.

[gakonst]

@hexonaut This should no longer be a prooblem since gakonst/ethers-rs#802. Feel free to remove forge clean from all your makefiles!