dtr-org / unit-e

A digital currency for a new era of decentralized trust
https://unit-e.io
MIT License
45 stars 15 forks source link

Run unit tests of Python scripts in contrib/ #1076

Closed cornelius closed 5 years ago

cornelius commented 5 years ago

There are a couple of scripts in contrib/ which have unit tests. These aren't run by CI yet. This change adds a step to the linter script which runs the unit tests before any scripts are run.

Posting as a WIP PR to test how Travis runs them.

frolosofsky commented 5 years ago

I liked first commit (4a2882bdc12a68b705c4a082b829677bd57fbd72) much more. The second's one intention could be achieved by set -e.

cornelius commented 5 years ago

I liked first commit (4a2882b) much more. The second's one intention could be achieved by set -e.

With set -e it would stop execution after the first failed test. As these tests are independent I would think it's better to execute them all and report overall failure, so you see all failures in one go without having to fix one failing test after each other.

scravy commented 5 years ago

Concept ACK

Gnappuraz commented 5 years ago

ConceptACK ec8aba2

cornelius commented 5 years ago

Rebased on master and force-pushed to get the fix which is supposed to make the tests pass.

scravy commented 5 years ago

@cornelius Is this still WIP?

cornelius commented 5 years ago

@cornelius Is this still WIP?

Yes, there is a pytest based test which still has to be added. Will push that shortly.

cornelius commented 5 years ago

Now everything should be in place. Removing WIP label.

scravy commented 5 years ago

Regarding set -e:

I am personally no fan of using this. It seems like a magic panacea, but it's not. Put simply, it often does not do what a lot of people suspect. Example:

set -e
x=0
let x++
echo "x is $x"

What's the result of it?

Blows up. Why? x is post-incremented and the value from before incrementing is returned. Which is zero. Which is treated by let as a false value. Which is converted into a non-zero exit code. Which blows up.

scravy commented 5 years ago

ACK 2da9d52f2fbda6d8728ca0bf2743633383fa35e2

I can see travis running the tests:

Screenshot 2019-05-08 12 52 44
Ruteri commented 5 years ago

The tests contrib/devtools/test_copyright_header.py and contrib/devtools/test_camel_to_snake.py are ran twice each (once via run_unit_test and once via pytest), so for now we could just run pytest, which also checks gitian.

cornelius commented 5 years ago

The tests contrib/devtools/test_copyright_header.py and contrib/devtools/test_camel_to_snake.py are ran twice each (once via run_unit_test and once via pytest), so for now we could just run pytest, which also checks gitian.

Good catch. This will be simpler. Will change it this way.