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

[isoltest] Add option to refer external files as sources #10475

Closed
axic opened this issue Dec 2, 2020 · 8 comments · Fixed by #11110
Closed

[isoltest] Add option to refer external files as sources #10475

axic opened this issue Dec 2, 2020 · 8 comments · Fixed by #11110

Comments

@axic
Copy link
Member

axic commented Dec 2, 2020

Currently the test format supports single or multiple files, but they all the sources have to be included in a literal form.

This idea came up on today's call: how about supporting a case pointing to a separate file. This would allow including larger external projects and still having the expectations visible, without the need to scroll down thousands of lines.

@axic axic added this to To do in Sol -> Yul CodeGen via automation Dec 2, 2020
@axic axic added this to New issues in Solidity via automation Dec 2, 2020
@axic
Copy link
Member Author

axic commented Dec 2, 2020

The current format is:

==== Source: A ====
contract A {}
==== Source: B ====
contract B is A {}

How about we make this extremely simple:

==== ExternalSource: someDirectory/someFile.sol ====
==== Source: B ====
contract B is A {}

The ExternalSource points to a relative directory, but rejects any path escaping the current directory (e.g. can descend to sub directories, but cannot go to parents).

@ekpyron
Copy link
Member

ekpyron commented Dec 2, 2020

The main problem I see here is: currently isoltest and the boost tests just traverse the entire directories consuming every single file as test file. Would those separate files we point to also be run as their own isolated tests? If not, how would we distinguish them? Placing them into an entirely different directory structure will make it very hard to figure things out... a special file name prefix/suffix/extension? Not sure...

@axic
Copy link
Member Author

axic commented Dec 2, 2020

Would those separate files we point to also be run as their own isolated tests? If not, how would we distinguish them?

Those files have no expectations in them so one could ignore such files?

@ekpyron
Copy link
Member

ekpyron commented Dec 2, 2020

Having no expectations at the moment still means they have to compile... we could however change that or have some comment-marker in them, indicating that they should not be run as individual test, yes.
(I expect not all of them will even be compilable on their own)

@chriseth
Copy link
Contributor

chriseth commented Dec 2, 2020

we can add a special comment in the first line of a file to be imported so it is ignored

@axic
Copy link
Member Author

axic commented Dec 2, 2020

Having no expectations at the moment still means they have to compile... [...] (I expect not all of them will even be compilable on their own)

That definitely would be the case as they frequently import files.

we can add a special comment in the first line of a file to be imported so it is ignored

Sounds good.

@axic
Copy link
Member Author

axic commented Dec 2, 2020

Should we allow remappings?

==== ExternalSource: ../zeppelin/SafeMath.sol=someDirectory/someFile.sol ====
==== Source: A ====
import "../zeppelin/SafeMath.sol";
contract B is A {}

@axic axic moved this from New issues to Implementation Backlog in Solidity Dec 2, 2020
@chriseth chriseth moved this from Implementation Backlog to Implementation Backlog Important in Solidity Dec 7, 2020
@axic
Copy link
Member Author

axic commented Mar 10, 2021

@aarlt this is the issue we discussed regarding the next priority for isoltest

Sol -> Yul CodeGen automation moved this from To do to Done Apr 27, 2021
Solidity automation moved this from Implementation Backlog Important to Done Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants