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

fix: display test results for tests with same name #1097

Merged
merged 4 commits into from
Mar 28, 2022

Conversation

onbjerg
Copy link
Member

@onbjerg onbjerg commented Mar 28, 2022

Companion PR: gakonst/ethers-rs#1087

Before and after:

~/Projects/github/onbjerg/failures master*
❯ forge test
[⠊] Compiling...
[⠒] Compiling 3 files with 0.7.0
Compiler run successful

Running 1 test for ContractTest.json:ContractTest
[PASS] testExample() (gas: 24964)

~/Projects/github/onbjerg/failures master*
❯ forge test
[⠊] Compiling...
[⠒] Compiling 3 files with 0.7.0
Compiler run successful

Running 1 test for /home/oliver/Projects/github/onbjerg/failures/src/test/Contract.t.sol:ContractTest
[PASS] testExample() (gas: 24964)

Running 1 test for /home/oliver/Projects/github/onbjerg/failures/src/test/Contract.t.2.sol:ContractTest
[PASS] testExample() (gas: 24964)

Remaining tasks

  • Make the path relative instead of absolute

Closes #1062
Closes #392

@onbjerg onbjerg added the T-bug Type: bug label Mar 28, 2022
@gakonst
Copy link
Member

gakonst commented Mar 28, 2022

@onbjerg ethers 1087 is now merged

@onbjerg onbjerg force-pushed the onbjerg/test-results-duplicate-contracts branch from 55a091a to 510f67d Compare March 28, 2022 19:23
@onbjerg
Copy link
Member Author

onbjerg commented Mar 28, 2022

Now looks like

~/Projects/github/onbjerg/failures master*
❯ forge test
[⠊] Compiling...
No files changed, compilation skipped

Running 1 test for src/test/Contract.t.sol:ContractTest
[PASS] testExample() (gas: 24964)
Test result: ok. 1 passed; 0 failed; finished in 399.52µs

Running 1 test for src/test/Contract.t.2.sol:ContractTest
[PASS] testExample() (gas: 24964)
Test result: ok. 1 passed; 0 failed; finished in 399.99µs

Cleaning up MultiContractRunner::source_paths is going to take more thought since the best way is probably passing around ArtifactId but there is a lot of dependent code. Would rather get this merged and clean that up when we clean up the test pass anyway

@onbjerg onbjerg marked this pull request as ready for review March 28, 2022 19:44
@onbjerg
Copy link
Member Author

onbjerg commented Mar 28, 2022

I have to adjust some tests before merge

@@ -3,7 +3,7 @@ pragma solidity >=0.8.0;

import "ds-test/test.sol";

contract DSStyleTest is DSTest {
contract DappToolsParityTest is DSTest {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was improperly named!

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the multi contract runner indeed is getting messy

@gakonst gakonst merged commit bb1716a into master Mar 28, 2022
@onbjerg onbjerg deleted the onbjerg/test-results-duplicate-contracts branch March 28, 2022 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
None yet
2 participants