Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forge build and forge test exit with No files changed, compilation skipped message #1244

Closed
2 tasks done
karmacoma-eth opened this issue Apr 9, 2022 · 17 comments · Fixed by gakonst/ethers-rs#1277
Closed
2 tasks done
Assignees
Labels
C-forge Command: forge Cmd-forge-build Command: forge build Cmd-forge-test Command: forge test T-bug Type: bug

Comments

@karmacoma-eth
Copy link
Contributor

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (9c24694 2022-04-08T00:20:01.154330+00:00)

What command(s) is the bug in?

forge test

Operating System

macOS (amd)

Describe the bug

I got into a state where every forge build and forge test command just returned some output like this, not actually building or testing anything:

forge test -vvv   
[⠢] Compiling...
[⠆] Compiling 72 files with 0.8.7
[⠰] Solc finished in 39.99ms
No files changed, compilation skipped

Even though I definitely have changed files. I added some assert(false) in my contracts, which should definitely blow up some tests, but I'm still getting the same.

Some things I've tried:

  • removing cache/
  • removing out/, but there is no out/ folder even after forge build
  • forge test --force, same thing
  • forge test --run-all, same thing
  • eventually, what resolved my issue was deleting a symlink I had created in src/, pointing to a folder in one of the submodules in lib/.

The context for the symlink is that I was trying to find a way to import a dependency that would work for both hardhat and foundry. I was able to deploy using hardhat with the symlink, but that turned out to break my foundry build/tests. I can go back to using the remapping, but that breaks hardhat deploys...

@karmacoma-eth karmacoma-eth added the T-bug Type: bug label Apr 9, 2022
@karmacoma-eth
Copy link
Contributor Author

I tried doing a minimal repro of the faulty setup here: https://github.com/karmacoma-eth/foundry-symlink-issue/pull/1/files

... but I am not actually able to reproduce 🤨

@mattsse mattsse self-assigned this Apr 9, 2022
@onbjerg onbjerg added Cmd-forge-test Command: forge test C-forge Command: forge Cmd-forge-build Command: forge build labels Apr 17, 2022
@aathan
Copy link
Contributor

aathan commented Apr 21, 2022

I think what may be happening is a combination of a forge feature and a bug: (1) Files are not recompiled if the content hasn't changed, even if the timestamp is newer than compilation outputs (2) If an error occurs during compilation, that file is still marked "not dirty" and won't be recompiled again until its contents change.

This can lead to unexpected results, where you have an syntax error in a contract, and then don't save the file after fixing it, and run the forge test command again, and get No files changed and/or it does recompile other files you did change but not the one file with the error you didn't save ... and then you get no test run and no output.

E.g., Test.sol is a test for Tested.sol. Both have syntax errors. forge test reports the errors. You fix Tested.sol. Next forge test compiles Tested.sol but not Test.sol and does not run the test...

@mattsse
Copy link
Member

mattsse commented Apr 22, 2022

thanks so much for looking into this, this makes sense, will try to confirm and fix!

@aathan
Copy link
Contributor

aathan commented Apr 22, 2022

While you are at it, please consider adding a feature/flag to force recompilation of units that have previously emitted warnings. --force is sometimes too aggressive but if you're trying to clean up warnings, you have to basically keep recompiling the world.

@mattsse
Copy link
Member

mattsse commented Apr 22, 2022

maybe a strict mode?

I think we need to cache the errors as well, which is probably a good idea anyway.

@aathan
Copy link
Contributor

aathan commented Apr 22, 2022

Whatever works for you. The usecase is to make cleaning up warnings not a pain.

@mattsse
Copy link
Member

mattsse commented Apr 22, 2022

but cleaning up warnings always involves modifying the file?
so they should get recompiled then?

@aathan
Copy link
Contributor

aathan commented Apr 23, 2022

I think you're assuming that these ephemeral warnings are saved somewhere by the foundry user.

Not every workflow involves an editor or IDE that has parsed and saved all the errors somewhere for the user to see later. Sometimes people issue a forge build or forge test specifically to cause the compiler to emit the same warnings they saw the last time they ran that command. If you implement a strict mode, and those files have not been touched, they should still invoke the compiler, so that the same messages will be emitted.

Imagine a terminal based workflow where the user has cleared the terminal and needs to regenerate the warnings. As is, the user doesn't even know which files to touch and worse it's not enough to just touch them, the user has to make a content change.

@aathan
Copy link
Contributor

aathan commented Apr 24, 2022

$ forge test -vvv --fork-url http://127.0.0.1:8545/ -m testDeployAndBuy
[⠒] Compiling...
[⠘] Compiling 2 files with 0.8.13
[⠃] Solc finished in 569.18ms
Compiler run successful
$ forge test -vvv --force --fork-url http://127.0.0.1:8545/ -m testDeployAndBuy
[⠒] Compiling...
[⠔] Compiling 45 files with 0.8.13
[⠒] Solc finished in 17.46s
Compiler run successful (with warnings)

Running 1 test for src/test/Contract.t.sol:ContractTest

After foundryup earlier today. Not sure if that installs latest nightly. Also, maybe my cache was in a weird state from prior version but I don't think so (because I did a --force using latest after installing latest). I'll try to catch the sequence of events that leads to this. In case it's not obvious, the problem is that I asked for a test to be run, but got silent nothing, until I asked again with --force. If the -m search finds nothing, it could be because there are dirty files that didn't get recompiled. Maybe at the least a message should be emitted that no matching test was found?

@mattsse
Copy link
Member

mattsse commented Apr 25, 2022

thank you for your insights here!

I definitely found an issue with the cache file that could lead to such a state, I wasn't able to reproduce this 1:1 but I think it's related.

@aathan
Copy link
Contributor

aathan commented Apr 26, 2022

I would definitely add some output on non-matched -m someTestName ... seems to me if someone is trying to run a specific test and the test isn't found, this should be considered an error. Right now forge is silent (but I didn't check the return code/value of the process).

That in and of itself may be worth a seperate Issue.

@mattsse
Copy link
Member

mattsse commented Apr 26, 2022

I would definitely add some output on non-matched -m someTestName ... seems to me if someone is trying to run a specific test and the test isn't found, this should be considered an error. Right now forge is silent

that's a good idea for UX improvement, wdyt @onbjerg? we should track this separately I think

@onbjerg
Copy link
Member

onbjerg commented Apr 26, 2022

We should track separately. Not sure if we want to error with exit code 1, but we should at least print the "x tests run; x ignored" etc. message.

@aathan
Copy link
Contributor

aathan commented Apr 28, 2022

There is still a bug here. If forge build --force yields an error a subsequent forge built without --force will not recompile the failed file!

@mattsse
Copy link
Member

mattsse commented Apr 28, 2022

thanks, any way to reproduce this?

@aathan
Copy link
Contributor

aathan commented Apr 28, 2022

As described. Make a project with 2-3 files in it. Create a syntax error in one of the files. forge test/build --force. Your next forge test/build (without the force) won't rebuild the file that had the error if that file hasn't been touched.

Maybe there's a philosophical issue here. Personally I expect if a file has errors, the build command should retry the build irrespective of whether it thinks the compilation source changed. Warnings may deserve a --force to rebuild. Better would be the --strict you previously suggested, where only the files with warnings would be rebuilt (vs all files).

@mattsse
Copy link
Member

mattsse commented Apr 28, 2022

thanks, will investigate asap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-build Command: forge build Cmd-forge-test Command: forge test T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants