OmniLayer / OmniJ

OmniLayer for Java, JVM, and Android
Apache License 2.0
133 stars 89 forks source link

Highlight how easy it is to run the tests #70

Closed dexX7 closed 9 years ago

dexX7 commented 9 years ago

I started to use the spock test suite way too late, because I initially assumed it would be a pain to setup, and I hadn't any idea how the gradle tasks work, or that the bash scripts sort of take care of the startup, and it was further not 100 % clear how the binary, the datadir and bitcoin.conf fit together, or that certain RPC credentials within the config were required. There was also the bad taste of some older Master Core 0.0.8 regtests, which looked like "one-click" scripts, but it turned out they used my default bitcoin.conf, and executed the tests on the current network (fortunally this was testnet at that time!).

@msgilligan convinced me to give it a try, after mentioning it really is just cloning the repo, and starting the tasks.

In a longer email conversation with @zathras-crypto I tried to explain how to setup IntelliJ IDEA and run the tests from within the IDE, but it's obvious, that this isn't a great way to hover into the waters either.

Now, actually it takes only four lines to get started, no scripts, no config, and no IDE, assuming Omni Core was already built:

# start omni core in regtest mode
~/omnicore$ ./src/qt/bitcoin-qt -datadir=/tmp -server -rpcuser=bitcoinrpc -rpcpassword=pass -rpcallowip=127.0.0.1 -regtest -txindex

# clone and run the tests
~/$ git clone https://github.com/OmniLayer/OmniJ.git
~/$ cd OmniJ
~/OmniJ$ ./gradlew omnij-rpc:regTest

Instead of using the QT client, this works of course also with bitcoind and in daemon mode:

~/omnicore$ ./src/bitcoind -datadir=/tmp -daemon -server -rpcuser=bitcoinrpc -rpcpassword=pass -rpcallowip=127.0.0.1 -regtest -txindex

... and to stop bitcoind daemon afterwards:

~/omnicore$ ./src/bitcoin-cli -datadir=/tmp -rpcuser=bitcoinrpc -rpcpassword=pass -regtest -rpcwait stop

So where I'm going? I'm not sure, where this project is heading (e.g. testing or mobile applications), but in my opinion it would probably be helpful, if the README.md is updated to have a straight forward "Getting started" section at the top.

Further, it would be awesome, if there were a quick instruction how to run a single test (I assume this is possible), instead of the whole test suite.

@zathras-crypto: did you give it a try in the meantime? If not, what was the reason to hesitate, and if so, what would you say would have made it easier for you to start?

msgilligan commented 9 years ago

OmniJ has 6 major categories of functionality:

  1. RPC client and command-line tools for Bitcoin Core
  2. Functional tests and test support libraries for Bitcoin Core
  3. RPC client and command-line tools for Omni Core
  4. Functional tests and test support libraries for Omni Core
  5. Omni protocol core data types, e.g. CurrencyID, OmniAmount, Ecosystem, etc.
  6. Omni protocol integration with bitcoinj and transaction signing (for Android and/or server apps)

These categories are currently split between three modules: bitcoin-rpc, omnij-rpc, and omnij-core. Over time it is likely that there will be more modules, but I prefer to keep the number as small as practical.

These six categories are inter-related and for most applications more than one is required. I'd like to see (1) and possibly (2) submitted and accepted by bitcoinj because I think the greater Bitcoin community will find them useful. I haven't suggested this on the bitcoinj mailing list yet -- I'd like to do a little more cleanup on the code first, but more importantly, I'm still refactoring code between the various modules. I'd like the boundaries between the modules to be better defined before splitting them off into another project.

I think the remaining items should remain in a single git repository and continue to be built with a Gradle multi-project build. We can split things into new modules when it makes sense (e.g. is needed to reduce library size or better factor dependencies.)

So one question (as the title of this issue implies) is how to produce documentation that allows users to quickly understand the various usage scenarios (e.g. Functional testing, Android wallet, exchange server, cryptocurrency trading bot, etc.) and which components are needed. I'm pretty happy with the current README though we might consider adding a table of contents (which can be automatically generated with Asciidoctor) I'm certainly open to any other suggestions for how the README could be improved.

I'd also like to produce a complete reference manual using AsciiDoctor with multiple chapters, a table of contents that can be accessed on the web or downloaded as a PDF. A well-written introduction that tells people which chapters they need to read for their use case and 'quick start' sections and/or sample apps for each of the major configurations (e.g. Spock integration tests, Android app, command-line tool, JVM-based server app) would also be helpful. In every case the steps should be as simple as:

  1. Start Omni Core (if needed)
  2. git clone <parameters>
  3. cd <directory>
  4. ./gradlew <task>

Once this manual is produced we may be able to shrink the README a little by providing links into the manual.

Thoughts?

zathras-crypto commented 9 years ago

@zathras-crypto: did you give it a try in the meantime? If not, what was the reason to hesitate, and if so, what would you say would have made it easier for you to start?

It's something that I've been 'meaning' to do for some time. The most open way I could put it is that of my lack of experience (in both Java and software testing).

You guys have made it incredibly easy to run the tests (I think it's just 4 commands IIRC) but I see that as limited in value for my own workflow because it already happens elsewhere (you guys, CI etc) - what I personally wanted to do was get a greater understanding of what they test, how they go about doing testing said 'what' and so on.

In a nutshell I've simply never taken time out to dive into the testing side of things. I've been fortunate to have you guys there for that (for which I'm very appreciative) which has allowed me to focus on the development side.

Perhaps I'm not doing a very good job explaining myself, tl:dr; I want to understand the testing process and approach, not just the results (we have things like CI for that) and I've yet to take a 'time out' to do so.

It is getting more and more relevant for me though - I will say that! Perhaps an example will help - take my auditor branch, that (as far as I can tell) is working perfectly for me, but the 'test-bitcoin' stuff is shutdown by the auditor when the state deviates from the audit cache. I need to understand what 'test-bitcoin' is doing (no clue!) to figure out what's going wrong.

mastercore_init()TESTNET, line 2524, file: mastercore.cpp
CMPTradeList(): OK, line 403, file: mastercore.h
CMPSTOList(): OK, line 360, file: mastercore.h
CMPTxList(): OK, line 460, file: mastercore.h
[Snapshot] Exodus balance: 0.00000000
Auditor initialized
[Initialized] Exodus balance: 0.00000000
Auditor has detected inconsistencies when attempting to update the total amount of tokens for property 1
Type:increase  Amount cached:0  Amount changed:-9919  Amount updated:-9919  Amount state:0

FAIL: test/test_bitcoin

TL:DR; I've had the good fortune of you guys running testing and haven't prioritized it myself. I see less value in running the tests than I do in understanding them. It's probably about time I started trying to understand them :)

msgilligan commented 9 years ago

@zathras-crypto, I understand your perspective. There is definitely a need for some better documentation for the purpose of understanding how the tests work and what they are actually testing.

I have a few things in mind:

  1. Improve comments in the tests themselves
  2. Write introductory/overview documentation that embeds relevant tests.
  3. Use alternative test report generators that provide more information
  4. Provide links back from failed (or successful) tests to the test source.

I'm not sure what can be done with off-the-shelf tools, but (1) is straightforward and (2) is easy enough to do with standard AsciiDoctor tools. (3) May be provided by the Spock Reports Extension. (4) May be possible with custom templates for the Spock Reports Extension.

dexX7 commented 9 years ago

@zathras-crypto: You guys have made it incredibly easy to run the tests

Ah, thanks for clearing this up. I created the topic under the impression that this step wasn't clear.

@msgilligan: I like the general plan, and I think the biggest hurdle now is getting started with the IDE, and learning about the project structure, but probably not so much with the actual tests.

Spock and Groovy are expressive, and, unless actions are encapsulated, like createFundedAddress, or it comes to data driven tests, which may not be that straight forward, the commands usually match RPC commands, which should be familiar to those using and testing Omni Core.

But this brings up an interesting point: we should probably try to avoid hiding what's happening:

createFundedAddress is telling, but I'm thinking about something like getTransactionMP vs. gettransaction_MP, where it might cause confusion, why in this particular case another name was used. If I were not familiar with the framework, I'd ask myself: "should I use send_MP or sendMP, after reading this..?" (or well, maybe not, given that I could look it up, but I think it's clear where I'm going ...)

Based on the above, I think it would be valuable:

  1. To create a very simple test, as introduction to the framework, the language, and as starting point for the creation of additional tests.
  2. To create a setup guide, ideally with pictures for each step, starting at the very beginning (namely JDK + IDEA), or providing links to external resources. This might be over the top, but even with some background in the field, there are some steps, which are not obvious, unless one is familiar with it, and this starts with having no setup routine for IDEA, but only ../bin/idea.sh.
msgilligan commented 9 years ago

getTransactionMP vs. gettransaction_MP ... why in this particular case another name was used?

The plain-Java OmniClienthas camel-case names, Java method-overloading, and other slight variances from the standard RPCs. The (incomplete) Groovy-based OmniCLIClient is intended to use Groovy dynamic dispatch and default arguments to create code that looks very much like the Bitcoin/Omni RPCs. OmniCLIClient will use lower-case names with the appropriate _MP or _Omni suffixes.

The idea being that people developing plain-Java servers, Android clients, etc. would use the Java clients and the Spock tests would convert to the "CLI" clients when they are finished. See Issue #64.

msgilligan commented 9 years ago
  1. To create a very simple test, as introduction to the framework, the language, and as starting point for the creation of additional tests.

The two sample tests in the README are meant to server this purpose.

msgilligan commented 9 years ago

To create a setup guide, ideally with pictures for each step, starting at the very beginning (namely JDK + IDEA), or providing links to external resources. This might be over the top, but even with some background in the field, there are some steps, which are not obvious, unless one is familiar with it, and this starts with having no setup routine for IDEA, but only ../bin/idea.sh.

The setup guide is a good idea, but first (I think) we should try to configure the gradle idea so that it does a better job configuring IDEA -- that should take much of the pain out. I also think it's more important to create documentation for people to read the test source code and understand the results. I think that's more important to people like @zathras-crypto, @CraigSellars , @marv-engine than being able to run IDEA.

dexX7 commented 9 years ago

There is currently no specific item to discuss/to do, so I'm going to close this.

msgilligan commented 9 years ago

I think the next step on this is to slim-down the README and start serious work on the OmniJ Developer's Guide