search

Consensys / truffle-security

MythX smart contract security verification plugin for Truffle Framework
https://mythx.io
124 stars 28 forks source link

On cli set return code to 0 if no error #118

Closed rocky closed 5 years ago

rocky commented 5 years ago

This should also assist calling shell scripts that might be used in Ci testing.

A possibly slick idea is to add one for every reported errors. So if there is one warning and two errors the return code might be three. If however we've decided not to report warnings, then the return codee would be 2. Note that the maximum value of a return code is 255.

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 0.1 ETH (13.34 USD @ $133.43/ETH) attached to it.

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 12 months from now. Please review their action plans below:

1) tagomaru has been approved to start work.

I will work on it. I know truffle-security logic since i already contributed to it.

Learn more on the Gitcoin Issue Details page.

tagomaru commented 5 years ago

A possibly slick idea is to add one for every reported errors. So if there is one warning and two errors the return code might be three. If however we've decided not to report warnings, then the return codee would be 2. Note that the maximum value of a return code is 255.

I think this ideas may be better to implement on out of return code. CI users may want to hook other problem such as timeout. That is why I would propose to return 1 always at the below cases.

Any ideas?

rocky commented 5 years ago

All of this is good stuff but will be in a separate gitcoin bounty, where we tackle a complete CI solution head on. Right now the main thing that is needed is a nonzero return code when there is any sort of problem.

Threshhold setting and what constitutes a problem again will be in a separate gitcoin challenge.

rocky commented 5 years ago

It occurs to me that this might be a little bit tricker than I had thought because this is getting run from inside truffle.

If run from a truffle development console, then we can't just exit. If not then we could possibly use process.exit(), but better would be to do whatever it takes to get truffle to return non-zero after its promise finishes.

rocky commented 5 years ago

I've asked on various truffle groups about what the proper thing to do is, or if this is supported.

However for expediency, let's just call process.exit() if environment variable CI is set. I checked both TravisCI, CircleCI, and Appveyor. By default, they all set this. Feel free to check other CI environments.

What I don't like about the enviornment variable name CI is this isn't strictly a CI thing, it is anything that exec's in a subrprocess truffle-security and checks the return code. But on the other hand, none of those CI systems have a common default environment variable set other than CI.

@tagomaru What do you think?

tagomaru commented 5 years ago

It occurs to me that this might be a little bit tricker than I had thought because this is getting run from inside truffle. If run from a truffle development console, then we can't just exit. If not then we could possibly use process.exit(), but better would be to do whatever it takes to get truffle to return non-zero after its promise finishes.

Yes, i know it. I think there is no way to use anything other than process.exit (). I do not like process.exit () in terms of best practice, though.　

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 0.1 ETH (13.79 USD @ $137.94/ETH) has been submitted by:

  1. @tagomaru

@b-mueller please take a look at the submitted work:

gitcoinbot commented 5 years ago
Octo Kitteh ⚡️ A *Octo Kitteh* Kudos has been sent to @tagomaru for this issue from @b-mueller. ⚡️ Nice work @tagomaru! Your Kudos has automatically been sent in the ETH address we have on file.
gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 0.1 ETH (13.75 USD @ $137.52/ETH) attached to this issue has been approved & issued to @tagomaru.

tagomaru commented 5 years ago

see #132