Skip to content

Extract examples from documentation and run tests on it #2553

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 13, 2017
Merged

Extract examples from documentation and run tests on it #2553

merged 6 commits into from
Jul 13, 2017

Conversation

axic
Copy link
Member

@axic axic commented Jul 10, 2017

Fixes #2552.

@axic axic force-pushed the extract-docs-tests branch 2 times, most recently from cbe95c6 to a3078e3 Compare July 10, 2017 22:24
axic
axic commented Jul 10, 2017
View reviewed changes
test/cmdlineTests.sh Outdated
do
echo "trying $f"
set +e
output=$("$SOLC" "$f" 2>&1)
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 is a copy of the previous test block. @chriseth if you could transform it into a bash function that could simplify it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want these to be more strict, e.g. require no warnings?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we could aim for that (with the exception of the pre-release compiler warning).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, but I won't add that to this pull request for now...

chriseth
chriseth reviewed Jul 11, 2017
View reviewed changes
scripts/isolate_tests.py

def write_cases(tests):
for test in tests:
open('test_%s.sol' % hashlib.sha256(test).hexdigest(), 'wb').write(test)

if __name__ == '__main__':
path = sys.argv[1]
docs = False
if len(sys.argv) > 2 and sys.argv[2] == 'docs':
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just do both all the time?

Copy link
Member Author

Choose a reason for hiding this comment

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

They require a different source directory (argv[1]).

@chriseth chriseth self-assigned this Jul 11, 2017
@chriseth
Copy link
Contributor

Trying to convert to a function.

@axic
Copy link
Member Author

axic commented Jul 11, 2017

Fails on paths:

Compiling all examples from the documentation...
trying test_12a2345f60bc89656aca0f44ee3327794490078cff6f15cdcb652a653c8da530.sol
scripts/../test/cmdlineTests.sh: line 64: scripts/../test/../build/solc/solc: No such file or directory

@chriseth
Copy link
Contributor

Actually I think we should first merge #2522

@axic axic mentioned this pull request Jul 12, 2017
Closed
@chriseth chriseth force-pushed the extract-docs-tests branch from a3078e3 to 33c0a08 Compare July 12, 2017 17:07
axic
axic commented Jul 12, 2017
View reviewed changes
test/cmdlineTests.sh Outdated
@@ -28,27 +28,86 @@

set -e

REPO_ROOT="$(dirname "$0")"/..
REPO_ROOT=$(cd "$REPO_ROOT" && pwd)
Copy link
Member Author

@axic axic Jul 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

How is this working?

Copy link
Contributor

Choose a reason for hiding this comment

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

$(...) executes the command(s) inside the parantheses and returns the output of that command as a value. && just means "only execute the next command if the previous command succeeded" i.e. exactly as the && in C. pwd prints the (absolute) path to the current directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean it references REPO_ROOT which is just an empty string at that point, so effectively it does cd and doesn't changes the current directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, now I see what you mean! Sorry, will fix it.

axic
axic commented Jul 12, 2017
View reviewed changes
docs/contracts.rst Outdated
Such contracts cannot be compiled (even if they contain implemented functions alongside non-implemented functions), but they can be used as base contracts::
// Such contracts cannot be compiled (even if they contain
// implemented functions alongside non-implemented functions),
// but they can be used as base contracts::
Copy link
Member Author

@axic axic Jul 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I think we're better off keeping this text unchanged and duplicating the Feline contract here.

axic
axic commented Jul 12, 2017
View reviewed changes
docs/security-considerations.rst Outdated
::

pragma solidity ^0.4.0;
// Now someone tricks you into sending ether to the address of this attack wallet:
Copy link
Member Author

Choose a reason for hiding this comment

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

I think we could keep the original text and add the interface of transferTo.

@chriseth
Copy link
Contributor

Addressed all comments.

axic
axic commented Jul 13, 2017
View reviewed changes
docs/security-considerations.rst Outdated
// Now someone tricks you into sending ether to the address of this attack wallet:
Now someone tricks you into sending ether to the address of this attack wallet::

interface TxUserWallet {
Copy link
Member Author

Choose a reason for hiding this comment

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

pragma is missing here

@axic
Copy link
Member Author

axic commented Jul 13, 2017

Changing this to fix the above.

@axic axic force-pushed the extract-docs-tests branch from 4f2d5ca to 62247cd Compare July 13, 2017 15:13
@axic
Copy link
Member Author

axic commented Jul 13, 2017

@chriseth this is ready from my side

@axic axic changed the title Extract examples from documentaiton and run tests on it Extract examples from documentation and run tests on it Jul 13, 2017
@chriseth chriseth force-pushed the extract-docs-tests branch from 62247cd to a8d78bb Compare July 13, 2017 19:47
@axic
Copy link
Member Author

axic commented Jul 13, 2017

@chriseth you need to approve this in order to merge

@chriseth chriseth merged commit 63bf0f6 into develop Jul 13, 2017
@axic axic deleted the extract-docs-tests branch July 13, 2017 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chriseth chriseth chriseth left review comments

Assignees

@chriseth chriseth

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@axic @chriseth