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

Testing: sharness environment #363

Merged
merged 8 commits into from Feb 25, 2019

Conversation

Projects
None yet
3 participants
@0xGabi
Copy link
Member

commented Feb 15, 2019

Sharness setup and some documentation on how to write tests.

We will add the first tests the next couple of days, we invite the community to help. The current commands we would like to test are listed in #358

# consider doing a graceful shutdown with -INT (-2) (interrupt - same as CTRL+C)
# or using -KILL (-9) if a clean termination does not work
test_expect_failure "'aragon ipfs' can be terminated" '
kill -9 -$PGID

This comment has been minimized.

Copy link
@0x6431346e

0x6431346e Feb 21, 2019

Member

While learning bash is quite fun... should we reconsider using sharness?

A seemingly easy task like killing a background process: aragon ipfs, proves to be very time consuming. 👇
After looking through ipfs' examples, the idea was simple:

> aragon ipfs &
> PID=$!
> kill $PID

The issue here was that kill $PID would only kill the shell process that invoked node, leaving node and ipfs running.
The solution is using kill -- -$PGID which is the syntax for killing a process group id. Luckily for us the PID and PGID are identical and retuned by $!:
image

Even though it works well from the terminal 🎉, with sharness, it proves a bit more difficult:

  1. The PID is not identical to the PGID
  2. The group of which the PID returned by $! is part of includes the sharness process, so not only it kills the node+ipfs processes, but also the test 😅

image

What we could do is filter the node processes that are part of the PGID, but it feels like this should be easier (other tests too - not only this specific example).

  1. Doesn't scale well - writing tests and maintaining them requires in-depth knowledge of shell scripting
  2. Not very portable - some tricks like process substitution, used in making stuff work, are not POSIX-compatible

How about we write these tests with with node and ava? @0xGabi @sohkai

  • Less effort: a contributor wouldn't need bash knowledge to write these tests, just javascript
  • Easier to read (IMO)

grep -m 1 "running" <(tail -f out_file)
vs

fs.watch('./output', (eventType, filename) => {
  fs.readFile('./output', function (err, data) {
    if(data.includes('running')) {
      test.pass()
    }
  })
})
  • We can leverage libraries. Example: use web3.js with localhost:8545 to test that ganache is working correctly, make calls to contracts like ENS to check if they're setup correctly, etc.

I'm not 100% sure, but I think we could do with node anything we would with bash.

This comment has been minimized.

Copy link
@sohkai

sohkai Feb 24, 2019

Member

0x6431346e Looking at these tests, it does seem much more maintainable if we just use node tooling rather than try and learn bash intricacies. Most people are not that well versed with shells, including those on the team, and this looks to be more of a nightmare to maintain than it would help 😅.

A few alternatives:

This comment has been minimized.

Copy link
@0xGabi

0xGabi Feb 25, 2019

Author Member

Yes, make sense. @0x6431346e ava seems the easy choice here. @sohkai Is there a reason you chose that framework over other options?

I would like us to consider Jest for aragonCLI cause test coverage right now is almost 0 and seems a really flexible framework to experiment with.

In the less likely case we want to stick with TAP there is node-tap as well

This comment has been minimized.

Copy link
@sohkai

sohkai Feb 25, 2019

Member

nixt supports other test runners; it simply might make some interactions with running CLI commands easier (instead of doing it ourselves like in the first two suggestions).

jest is nice as it supports snapshots.

This comment has been minimized.

Copy link
@sohkai

sohkai Feb 26, 2019

Member

Found out that ava also supports snapshot testing, so FWIW we could just use ava 🤷‍♂️

This comment has been minimized.

Copy link
@0xGabi

0xGabi Feb 26, 2019

Author Member

Thanks Brett!

@0xGabi 0xGabi referenced this pull request Feb 25, 2019

Closed

Migrate tests to node #376

@0xGabi 0xGabi merged commit 068afde into aragon:master Feb 25, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@0xGabi 0xGabi deleted the 0xGabi:add-sharness branch Feb 25, 2019

@0xGabi 0xGabi referenced this pull request Feb 25, 2019

Closed

E2E tests with sharness #358

2 of 4 tasks complete

@0xGabi 0xGabi added this to the aragonCLI v5.4 milestone Feb 25, 2019

0xGabi added a commit that referenced this pull request Feb 25, 2019

Testing: sharness environment (#363)
* Testing: sharness environment setup

* Add sharness tests to help and version

* Test: Add create-aragon-app and aragon init test

* Test: Update create-aragon-app and init tests

* Add aragon ipfs

* Fix attempt for aragon_ipfs test

0xGabi added a commit that referenced this pull request Feb 28, 2019

Monorepo structure (#325)
* Fix setPermissions in dao install (#303)

* Fix app install command (#287)

Add NO_MANAGER (0x00..) to set permission task on app installation. Before
ANY_ENTITTY (0xff...) was being used, so permissions were being blocked.

* Upgrade version to 5.2.2

* Update ganache-core to 2.3.2 (#302)

* Update ganache-core to 2.3.2

* Add package-lock.json to make CI pass

* Fix bug in `aragon dao acl` command (#304)

Fix printing bug in `aragon dao acl`

* Create CONTRIBUTING.md

* Add documentation about .ipfsignore

close #135

* Update README.md

* Update README.md

* Add help text to `aragon dao install --app-init` option (#312)

If install command doesn't find initialization method, it won't
initialize the newly installed app, but this is not obvious from the
help instructions.
Although it's "dangerous", sometimes it's useful to install an app
without initializing. For instance, Token Manager: to initialize it
the token must be provided, and this token must have this new Token
Manager app as controller. Therefore token controller must be changed
in between creation and initialization of Token Manager app.

* Add filter for files passed as parameter to not be ignored (#313)

* add filter for files

* consider ignore files without eol at the end

* Add ipfs-check to 'aragon new dao' (#305)

* lerna import aragon-cli and create-aragon-app

* delete old root files

* delete files from package dont needed localy

* update lock files after bootstrap

* Delete packages to reimport again

* Initial commit

* First Commit

* Add README

* add yargs default command

* 1.0.1

* Update README.md

* Update README.md

* 1.0.2

* Update README.md

* 1.0.3

* Update README.md

* 1.0.4

* Update: prepare-template for environments

* 1.0.5

* Fix: changes after review

* 1.0.6

* Fix: remove test from documentation lint

* Initial

* Rename package to @aragon/cli

* Merge pull request #16 from aragon/ens-address

Add ENS registry address flag

* 1.1.0

* Update authorship

* Merge pull request #20 from aragon/scaffolding

Support scaffolding

* 1.2.0

* Create LICENSE

* Merge pull request #21 from aragon/rename-module-to-app

Rename module to application

* 1.2.1

* Update examples in README

* Merge pull request #22 from aragon/refactor

Initial refactor

* Update usage in README

* Bump yargs to 10.1.x

* Refactor CLI entrypoint to use middlewares

* Merge pull request #28 from macor161/node-version

Add node version dependency

* 2.0.0

* hackathon fix #1

* 2.0.1

* Merge pull request #30 from aragon/os3

[WIP] Parse functions and new roles and add onlyArtifacts flag to publish

* Merge pull request #36 from aragon/bin-rename

Rename bin from aragon-dev-cli to aragon

* Merge pull request #40 from gasolin/arapp

Rename module.json to arapp.json, fix #29

* Merge pull request #38 from aragon/rinkeby-publish

make publish work on rinkeby

* Merge pull request #41 from aragon/pin-web3

Pin web3 to beta.26

* v2.0.5

* Merge pull request #42 from aragon/pin-ganache

Pin ganache-core to under 2.1.0

* v2.0.6

* Merge pull request #44 from aragon/fix-no-params

Solidity extractor: fix parameter parsing when no params exist

* Merge pull request #45 from aragon/fix-function-in-comment

Solidity extractor: fix usages of 'function' in comment flagging declarations

* v2.0.7

* Return content URI even if artifact doesn't exist

* Merge pull request #49 from aragon/grant

aragon grant

* Delete .node-version

* Delete wallaby.js

* Merge pull request #52 from aragon/cleanup-reporter

Clean up reporter

* Merge pull request #53 from aragon/cleanup-commands

Clean up commands

* Merge pull request #59 from aragon/apm.js

Use `@aragon/apm`

* Merge pull request #58 from aragon/better-ignore

Better ignore

* Add a brand new README

* Add configuration docs

* Merge branch 'beta'

* Fixing RPC flags

* Couple changes

* Version bump

* Fixes #72

* Bump version

* Fixes #88

* Fixes #80

* Fixes #77

* Groups APM ops into aragon apm subcommand (#87)

* Make DAO a more generic command group (#87)

* Added all apm subcommands

* Closes #87

* Added dao and apm aliases for command line. Not updating ganache snapshots anymore, rather automatically bumping the package version if using aragon run

* Bumping version

* Remove aragon-dev-cli entrypoint, finishing rename

* Rename aliases

* Fix init name issue, close #76

* Merge pull request #91 from aragon/deploy-command

Implement deploy command and use it everywhere + tons of low hanging fruit

* Add warning if wrapper port was already open. Close #75

* Requiring node >=8

* Merge pull request #94 from aragon/dao-install

New commands (dao new, dao install, dao upgrade, aragon ipfs) + aragon run refactor

* Publish to npm

* Merge branch 'master' of github.com:aragon/aragon-dev-cli

* 4.0.0

* Merge pull request #101 from aragon/update_readme

Update README

* Merge pull request #104 from aragon/wrapper-pin

Pin wrapper to master

* 4.0.1

* IPFS check option in publish and update to @aragon/cli 1.1.2

* 4.0.2

* Always publish manifest from project root

* 4.0.3

* Fix

* Merge branch 'master' of github.com:aragon/aragon-dev-cli

* Update web3 to 1.0.0-beta.34

* Merge master

* Merge pull request #112 from aragon/print-mnemonic-phrase

Print mnemonic phrase when running the dev chain

* 4.1.1

* Update README.md

* Bypass no artifact found for bare kit hardcoding ABI temporarily (#126)

Temp fix for

```
`Fetching kit bare-kit.aragonpm.eth@latest
→ Cannot find artifacts in APM repo. Please make sure the package is public ...`
```

This should make the bare-kit work regardless of whether my ipfs daemon is on lol

* 4.1.2

* Update package-lock.json

* Update @aragon/apm and pin 1.1.6

* 4.1.3

* Merge pull request #155 from jtremback/network-id-fix

janky network id fix

* Fix missing opn import (#157)

* Merge pull request #161 from aragon/pin-ethereumjs-wallet

Bump ganache-core version to ensure ethereumjs-wallet is pinned to 0.6.0

* 4.1.4

* Merge pull request #162 from PascalPrecht/fix/129

fix: don't use yarn for aragon init

* Merge pull request #164 from PascalPrecht/fix/init-cmd

fix(commands/init): throw meaningful error if project folder already exists

* Merge pull request #165 from aragon/cleanup-comment

Chore: Clean up commented log

* Merge pull request #144 from aragon/only-content-artifacts

Always generate artifact if there is a contract linked in the arapp file

* Merge pull request #169 from aragon/apm-dep

Add apm.js dependency

* Merge pull request #140 from aragon/custom-build

Implement custom build script in 'apm publish'

* Merge pull request #145 from aragon/publish-deploy-contract

Pass contract name specified in publish to deploy

* 4.1.5

* Merge pull request #170 from aragon/devchain-resets

Improve experience around devchain resets

* Merge pull request #178 from PascalPrecht/fix/commands/apm/publish-error-msg

fix(commands/apm/publish): fix link to version upgrade rules

* Merge pull request #176 from PascalPrecht/fix/commands/apm/grant

fix(commands/apm/grant): use updated CREATE_VERSION_ROLE hash

* Merge pull request #174 from PascalPrecht/uiux/commands/apm

uiux(commands/apm): show meaningful error messages

* Merge pull request #181 from aragon/wrapper-commit

Fix wrapper commit not being checked out and update wrapper commit

* 4.1.6

* Improve error message when provider is unaccessible (#185)

Improve error message when provider is unaccessible and support provider lazy loading

* Add react-kit alias for aragon init (#184)

* Remove apm alias and update readme references (#183)

* Remove apm alias and update readme references

* Add aragon package alias for aragon apm

* Add call command (#141)

* Add call command

* debug cleanup

* responded to comments

* Improve error message when provider is unaccessible (#185)

Improve error message when provider is unaccessible and support provider lazy loading

* Add react-kit alias for aragon init (#184)

* Remove apm alias and update readme references (#183)

* Remove apm alias and update readme references

* Add aragon package alias for aragon apm

* Minor clean up to exec command

* Added link to dev portal in README

* Update to latest @aragon/wrapper and display permission managers (#187)

* Update to @aragon/wrapper@2.0.0-beta.42

* Only show permissions in dao acl when there are allowed entities

* Display permission manager in dao acl command

* Added HTTP provider feature (WIP) (#171)

* Added HTTP provider option

* Rename http provider flags

* Update to @aragon/apm 2.0.1

* Small improvements to http provider

* Implement ACL write commands (#190)

* dao acl view: fix truncated proxy addresses

* dao exec: extract exec logic into handler for reusability

* aragonjs-wrapper: clean up unused code

* dao acl grant: Implement command

* dao acl create: Implement command

* dao acl revoke: Implement command

* dao acl set-manager: Implement command

* dao acl remove-manager: Implement command

* Exec handler clean up

* dao acl: allow role names

* Update to @aragon/wrapper 2.0.0-beta.43

* Fix wrapper connection ipfs settings

* Update package-lock.json

* 4.2.0

* Add core-js dependency

* 4.2.1

* Remove core-js dep and use babel-polyfill

* 4.2.2

* Make babel-polyfill dependency

* 4.2.3

* [WIP] Update to aragonOS 4 (#186)

Update to aragonOS 4

* Update wrapper version

* 5.0.0

* Install go-ipfs as dependency, remove the need for manual install (#205)

* Install go-ipfs as dependency, remove the need for manual install

* Typo fix

* Using spaces for ipfs.js

* Remove unused return value in ensureIPFSInitialized

* feat(commands/run): introduce --client-version and --client-port (#194)

* refactor: move aragon client version into package.json property

NPM's package files support custom properties for library specific usages.
This commit moves the aragon client version into a newly introduced
`aragon` properties section.

* refactor: rename WRAPPER_PATH, WRAPPER_PORT to CLIENT_PATH, CLIENT_PORT

* feat(commands/run): introduce --client-version and --client-port

This commit introduces a new option `--client-version` that can be used to
specify the Aragon client version to be used as discussed in #182. A version
can be any commit-ish values (hashes, branches, tags).

Example:

```
$ aragon run --client-version=fa2025232330bc3e4d3b792cab4bf44d754d33e6
```
Or

```
$ aragon run --client-version=dev
```

It also offers a new option `--client-port` to configure the port on which
the client is being served from.

Notice that this option is being ignored at the moment, which will be addressed
in a future commit. For more info: #198

Closes #182

* Removed homedir dependency (#208)

* feat(commands/run): introduce --client-path option (#200)

This commit enables users to pass a `--client-path` to the `run` command.
The path specifies the location to an already existing installation of the
Aragon client on the local machine, allowing developers to test custom versions
of the app.

Closes #199

* fix(apm/publish): check if artifact.json already exist (#175)

* fix(apm/publish): check whether artifact.json already exist

And throw if it doesn't. CLI users will have to generate the artifact.json
explicitly using `apm publish --only-artifacts`.

Closes #159

* feat(apm/publish): allow users to generate artifact if it doesn't exist

* Update @aragon/apm to 2.0.2 (#210)

* Using local dependencies by default, and falling back to global if not found (#207)

* fix(commands/run): ensure configured client wrapper port is used (#201)

Prior to this commit, using `WRAPPER_PORT` and changing it to a users needs
didn't have any effect as

a) the port has never been passed down to the Aragon client
b) Aragon client's script isn't prepared to retrieve values from the environment

This commit ensures that the port is set as an environment variable,
enabling Aragon client to take advantage of it.

Fixes #198

* Merge pull request #209 from aragon/remove-aragon-apps-dep

Moved @aragon/apps-* to devDependencies

* Remove unnecessary BROWSER env var in aragon run (#212)

* Merge pull request #216 from aragon/bugfix/215-update-wrapper-commit

Update Aragon Core to latest(526a19)

* Merge pull request #168 from PascalPrecht/feat/test-setup

refactor: move init command functions into lib and introduce tests

* Update deps

* 5.1.0

* Fix solidity extractor (#229)

* Fix solidity extractor when radspec strings have function calls

* Add string type

* Fix visibility modifier detection in multiline function declaration

* Add timeout for starting IPFS and better error handling (#211)

* Add timeout for starting IPFS and better error handling

* Always migrate on IPFS daemon start to avoid user prompt

* Bugfix/220 fix linting (#221)

* Fix linting command and autofix lint errors

* Manually fix lint errors

* Add babel-eslint and fix the ava config

* Add documentation lint

* Upgrade babel dependencies

* Fix identation + ipfs cors check

* Fix CORS check for ipfs

* Use existent artifact file if present (#226)

* 5.1.1

* fix bug in printing apps table (#236)

* apm publish: Improve publish feedback (#231)

* apm publish: Improve publish feedback

* apm publish: Print publish directory path on debug

* Add deployment artifacts to artifact.json (#232)

* Add deployment artifacts to artifact.json

* Clean up artifact generation

* Fetch artifacts from previous version if running in --only-content mode

* Fix crash when there are no compiler optimization settings

* Use gas price from truffle config file (#228)

* Use gas price from truffle config file

* Fix lint

* Enable environments in arapp.json (#230)

* enable environments in arapp.json

* suggested changes

* pass userApps directly to listApps

* fix merge conflict

* Fix skip network commands

* Minor bug fixes to publish command

* Fix

* remove licenses when preparing template (#234)

* Update to @aragon/wrapper 2.0.0 (#240)

* 5.2.0-beta.1

* Always alow to provide --network flag if running the contracts command (#247)

* Always alow to provide --network flag if running the contracts command

* 5.2.0-beta.2

* Fix exec command when run with multiple arguments (#241)

* Add --use-frame flag for using frame as the web3 provider (#242)

* Dont require artifact to exist after publishing (#250)

* aragon apm grant: support granting permissions to many addresses (#251)

* 5.2.0-beta.3

* dao acl *: Add middleware (#256)

* 5.2.0-beta.3

* dao acl: Add middleware

* 5.2.0-beta.4

* Fix IPFS dependency pinning to 0.4.17 (#260)

* Add origin header to frame provider (#258)

* Add origin header to frame provider

* Fix naming

* 5.2.0

* Merge pull request #262 from aragon/print_ens

Add ENS address output to `aragon devchain`

* 'wrapper' -> 'client' in logs (#269)

* Merge pull request #245 from perissology/ipfs-check

implement better ipfs running check

* Merge pull request #252 from perissology/acl-view

fix bug in acl view cmd

* Merge pull request #223 from jvluso/debug

Add silent and debug logging options

* Merge pull request #268 from aragon/0.6

Update to aragon 0.6 dependencies

* Merge pull request #280 from aragon/fix_ganache

Pin ganache version to 2.2.1

* Merge pull request #279 from galactusss/pre-commit

Add husky for pre-commit: lint-stage & pre-push: test

* Merge pull request #284 from aragon/apm-versions-verbose-error

Show more info when content cannot be found

* Support 'dao upgrade' and 'dao install' on live DAOs (transaction pathing) (#270)

* Append .aragonid.eth if needed

* Update to @aragon/wrapper 3.0.0 beta and add wsProvider to environment

* Add transaction pathing for dao install and dao upgrade

* Add wsProvider support for 'dao apps' and 'dao acl' commands

* chore: update license to Aragon Association (#292)

* chore: update license text (#293)

* Fix setPermissions in dao install (#303)

* Fix app install command (#287)

Add NO_MANAGER (0x00..) to set permission task on app installation. Before
ANY_ENTITTY (0xff...) was being used, so permissions were being blocked.

* Upgrade version to 5.2.2

* Update ganache-core to 2.3.2 (#302)

* Update ganache-core to 2.3.2

* Add package-lock.json to make CI pass

* Fix bug in `aragon dao acl` command (#304)

Fix printing bug in `aragon dao acl`

* Create CONTRIBUTING.md

* Add documentation about .ipfsignore

close #135

* Update README.md

* Update README.md

* Add help text to `aragon dao install --app-init` option (#312)

If install command doesn't find initialization method, it won't
initialize the newly installed app, but this is not obvious from the
help instructions.
Although it's "dangerous", sometimes it's useful to install an app
without initializing. For instance, Token Manager: to initialize it
the token must be provided, and this token must have this new Token
Manager app as controller. Therefore token controller must be changed
in between creation and initialization of Token Manager app.

* Add filter for files passed as parameter to not be ignored (#313)

* add filter for files

* consider ignore files without eol at the end

* Add ipfs-check to 'aragon new dao' (#305)

* deprecate aragon init (#323)

* Handle arapp.json parse errors (#330)

Testet! Great feature added

* Create a new ganache db for each version (#329)

* Change dao exec to use wsRPC (#338)

* Add global environments (#336)

* Print permissionless apps (#340)

* New command: change-controller (#339)

* Allow MiniMe token creation (#335)

* Pin ganache to 2.2.1 (#341)

* Use tx pathing for setPermissions (#334)

* Use tx pathing for setPermissions

* Fix setPermissions

* fix(dao install): update error message when installing app without roles (#343)

Rewords to "roles" as it's the key in the `arapp.json`. Also gives a bit more guidance as to how to find more information.

* Bump minor -> 5.3.0 (#342)

* Update client version to 0.6.2

* 5.3.1

* feat: upgrade aragon client (#345)

Includes windows-compatible scripts for aragon/aragon

* 5.3.2

* Pin web3 (#354)

* 5.3.3

* fix: Remove path and roles from being mandatory in arapp.json schema (#351)

* Fix gasLimit issue for dao new (#361)

* Fix gas on grant and publish (#366)

* Add staging as default environment (#368)

* aragon init: prepare template for environments

* Update: prepare-template for environments

* Fix: update lint test folder

* Fix: update ava tests

* Fix: ava test

* Fix: changes after review

* Implement `dao act` command for Agent app integration (#356)

* WIP: dao act command

* dao act: working

* dao act: cleanup

* Fix target explanation

Co-Authored-By: izqui <izqui97@gmail.com>

* Upgrade @aragon/wrapper (#359)

* Fix ABI import of the Kernel

* Fix ABI import of the Kernel

* Fix: Remove documentation lint for test

* Pin last @aragon/wrapper beta

* Update install.js

Add abi format

* Update package.json

Fix: lint rule typo

* Bump version to beta and remove package-lock.json

* Fix: version typo

* Testing: sharness environment (#363)

* Testing: sharness environment setup

* Add sharness tests to help and version

* Test: Add create-aragon-app and aragon init test

* Test: Update create-aragon-app and init tests

* Add aragon ipfs

* Fix attempt for aragon_ipfs test

* Update travis.yml to install last npm version

* pin v5.4 and update client version

* Clean redundant files

* Fix travis

* Disable npm ci when installing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.