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

Add extra documentation for running compiler tests on Windows #5181

Merged
merged 1 commit into from Oct 15, 2018

Conversation

Projects
None yet
5 participants
@Mordax
Contributor

Mordax commented Oct 10, 2018

Checklist

  • Code compiles correctly
  • All tests are passing
  • New tests have been created which fail without the change (if possible) //Doc change, not code change
  • README / documentation was extended, if necessary
  • [ ? ] Changelog entry (if change is visible to the user)
  • Used meaningful commit messages

Description

Hi there, new to the project and quite excited about Solidity. As a stubborn (and lacking other environment options) Windows user, I found that there was an important difference in the statement used for running Solidity tests, so I've added a note for Windows users on where they have to look to run the tests. Hopefully it'll save some time for some developers.

I've done my best to follow the practices laid out in the contributing docs, and I'm interested in further contributing documentation and other fixes. Let me know if I've missed something or forgot a protocol. I didn't update the changelog as I didn't really notice documentation being common, but I'm more than happy to add it in if need be.

@codecov

This comment has been minimized.

codecov bot commented Oct 10, 2018

Codecov Report

Merging #5181 into develop will increase coverage by 59.63%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #5181       +/-   ##
============================================
+ Coverage    28.38%   88.02%   +59.63%     
============================================
  Files          315      318        +3     
  Lines        31751    31907      +156     
  Branches      3799     3764       -35     
============================================
+ Hits          9012    28085    +19073     
+ Misses       22058     2551    -19507     
- Partials       681     1271      +590
Flag Coverage Δ
#all 88.02% <ø> (?)
#syntax 28.69% <ø> (+0.3%) ⬆️
@@ -82,6 +82,12 @@ To run a basic set of tests that neither require ``aleth`` nor ``libz3``, run
``./scripts/soltest.sh --no-ipc --no-smt``. This script will run ``build/test/soltest``
internally.

.. note ::
Those working in a Windows environment wanting to run the above basic sets without aleth or libz3 in Git Bash, you would have to do

This comment has been minimized.

@chriseth

chriseth Oct 10, 2018

Contributor

Thanks for the addition! Could you compress the paragraph a little? Also I think that people should know themselves that you have to build the tests first and can translate the / and \ path notations - shouldn't / work on windows in general nowadays?

This comment has been minimized.

@christianparpart

christianparpart Oct 10, 2018

Contributor

@chriseth to answer your windows-path-seperator question, the Windows Command Prompt only knows back-slash (\) but the PowerShell accepts both (/ and \).

This comment has been minimized.

@Mordax

Mordax Oct 12, 2018

Contributor

@chriseth Quick clarification - by compressing, do you want me to make this less wordy or remove spacing? I can also eliminate the command prompt line and just state to be mindful of slashes, because as @christianparpart said, depending on where you run them, certain slash directions don't work.

This comment has been minimized.

@chriseth

chriseth Oct 13, 2018

Contributor

just less wordy and probably good to keep the command - when I read such documentation, I mostly just look at the commands and if I want to know more, I read the text in addition to that

Those working in a Windows environment wanting to run the above basic sets without aleth or libz3 in Git Bash, you would have to do
a different command: ``./build/test/RelWithDebInfo/soltest.exe -- --no-ipc --no-smt``. Make sure you followed the `building from source <https://solidity.readthedocs.io/en/latest/installing-solidity.html#building-from-source>`_
and that your Cmake was successful. If you're running this in plain Windows Command line, use ``.\build\test\RelWithDebInfo\soltest.exe -- --no-ipc --no-smt``.

This comment has been minimized.

@christianparpart

christianparpart Oct 10, 2018

Contributor

RelWithDebInfo inclines that users did build in that mode on Windows, however, all others are supported too.

This comment has been minimized.

@christianparpart

christianparpart Oct 10, 2018

Contributor

This also applies to non-Windows development platforms. soltest.sh MAY also become superfluous since at least the --testpath logic was just lately merged into the soltest program itself, not requiring soltest.sh to do that magic anymore.

What is different on Windows however is how to access the aleth's IPC socket.

This comment has been minimized.

@chriseth

chriseth Oct 10, 2018

Contributor

soltest is still more convenient, since you do not have to care about splitting into boost and non-boost options and can use the --debug switch.

This comment has been minimized.

@Mordax

Mordax Oct 12, 2018

Contributor

@chriseth @christianparpart Should I keep the run test command or should I change up some flags?

@ChrisChinchilla

This comment has been minimized.

Contributor

ChrisChinchilla commented Oct 15, 2018

Thanks for the contribution @Mordax I am also very keen to make the onboarding process easier for everyone. I'm going to test this all myself on a Windows VM and will let you know if I have any suggestions.

@chriseth chriseth force-pushed the Mordax:doc-note-windows branch from 8a2d631 to f60251f Oct 15, 2018

@chriseth chriseth merged commit b965fd6 into ethereum:develop Oct 15, 2018

0 of 8 checks passed

ci/circleci: build_emscripten CircleCI is running your tests
Details
ci/circleci: build_x86_linux CircleCI is running your tests
Details
ci/circleci: build_x86_mac CircleCI is running your tests
Details
ci/circleci: docs CircleCI is running your tests
Details
ci/circleci: test_buglist CircleCI is running your tests
Details
ci/circleci: test_check_spelling CircleCI is running your tests
Details
ci/circleci: test_check_style CircleCI is running your tests
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@Mordax

This comment has been minimized.

Contributor

Mordax commented Oct 16, 2018

@ChrisChinchilla Awesome, let me know if you run into anything and I can confirm on my end.

@@ -82,6 +82,11 @@ To run a basic set of tests that neither require ``aleth`` nor ``libz3``, run
``./scripts/soltest.sh --no-ipc --no-smt``. This script will run ``build/test/soltest``
internally.

.. note ::
Those working in a Windows environment wanting to run the above basic sets without aleth or libz3 in Git Bash, you would have to do: ``./build/test/RelWithDebInfo/soltest.exe -- --no-ipc --no-smt``.

This comment has been minimized.

@axic

axic Oct 17, 2018

Member

Wouldn't it be better to rephrase this to show that soltest can be used directly without the script?

@ChrisChinchilla

This comment has been minimized.

Contributor

ChrisChinchilla commented Oct 17, 2018

@Mordax @chriseth I found some confusion with the various MS dependencies needed but couldn't quite pinpoint what I had forgotten to install. I may test again in the future. If you err on the side of install more than you need everything builds correctly.

@Mordax One question for you. I then looked at how to run tests under Windows. Have you had any success with those, and would you be willing to submit a PR maybe clarifying how Windows users can do that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment