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 integration tests and cross-platform test #561

Closed
onbjerg opened this issue Jan 24, 2022 · 9 comments
Closed

Fix integration tests and cross-platform test #561

onbjerg opened this issue Jan 24, 2022 · 9 comments
Assignees
Labels
D-chore Difficulty: chore P-normal Priority: normal T-meta Type: meta

Comments

@onbjerg
Copy link
Collaborator

onbjerg commented Jan 24, 2022

  • Cross-platform tests are failing because of directory separators and line endings on Windows
  • Integration tests are now passing
@onbjerg onbjerg added T-meta Type: meta P-normal Priority: normal D-chore Difficulty: chore labels Jan 24, 2022
@gakonst
Copy link
Member

gakonst commented Jan 24, 2022

For geb, the stop gap would be to use solc 0.6.7 and run it with --no-auto-detect, vs allowing it to fail the compilation step (might as well remove it in that case)

@onbjerg
Copy link
Collaborator Author

onbjerg commented Jan 24, 2022

I think we should pursue figuring out why the version detecting fails instead then. I think it might be a bug in solang-parser. When I tried running the integration test for geb locally with RUST_LOG=trace there were a lot of test files we couldn't parse because they use decimal numbers with exponents. Has been fixed in hyperledger-solang/solang#646

@gakonst
Copy link
Member

gakonst commented Jan 24, 2022

Specifically for Geb this is what I had seen
telegram-cloud-photo-size-2-5420255501171668713-y

there’s actually 2 bugs

  1. the contracts compiled with 0.8.11 outside of delegate.sol are actually all contracts with empty code in them so we should not be creating jobs where source has no code for compilation, wonder if we can check that somehow, they are full of comments
  2. delegate.sol gets used in both jobs

@brockelmore
Copy link
Member

we should likely add a hardhat repo as an integration test if at all possible. I know we dont expect 0 overhead for most cases but we should try to find one that "it just works" for and add it

@onbjerg
Copy link
Collaborator Author

onbjerg commented Jan 27, 2022

Updated OP to reflect the latest state of integration tests

@onbjerg
Copy link
Collaborator Author

onbjerg commented Jan 29, 2022

Update, we fixed what we could here and then I fixed geb tests upstream. Our integration tests are now passing!

@onbjerg onbjerg self-assigned this Jan 29, 2022
@gakonst
Copy link
Member

gakonst commented Jan 29, 2022

@onbjerg can we re-enable Geb but only have it run with solc 0.7.6?

Maybe we should add a flag called "--solc-version" which loads solc from PATH or SOLC_PATH (if it matches the requested version) or from ~/.svm/ or tries to install it via svm?

That way anyone could do "forge test --solc-version x.y.z" and it'd auto select that / download it if not present

@onbjerg
Copy link
Collaborator Author

onbjerg commented Jan 30, 2022

Yes, let's do that

@onbjerg
Copy link
Collaborator Author

onbjerg commented Jan 30, 2022

Going to keep this open until we re-enable geb

@gakonst gakonst closed this as completed in 2fecfa9 Feb 9, 2022
gakonst added a commit that referenced this issue Feb 9, 2022
bumps svm-rs.
fixes #525
fixes #684

driveby: re-enable geb integration test (fixed in gakonst/ethers-rs#866)
fixes #561
gakonst added a commit that referenced this issue Feb 9, 2022
bumps svm-rs.
fixes #525
fixes #684

driveby: re-enable geb integration test (fixed in gakonst/ethers-rs#866)
fixes #561
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-chore Difficulty: chore P-normal Priority: normal T-meta Type: meta
Projects
None yet
Development

No branches or pull requests

3 participants