Query tests

[greg-szabo]

Description

This is a first stab at testing the queries in the relayer-cli. It uses abscissa's acceptance testing method.

This method is hard to troubleshoot and configuration is not granular enough. It works fine, so we should use it until better methods are implemented.

The relevant issues are already closed (#130, cosmos/ibc-rs#130, #151) but testing was missing there.

---

For contributor use:

• Unit tests written
• Added test to CI if applicable
• Updated CHANGELOG_PENDING.md
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Updated relevant documentation (docs/) and code comments
• Re-reviewed Files changed in the Github PR explorer

[romac]

Not quite sure I understand the CI config changes, but the tests otherwise look good to me 👍

By the way, here's a quick and dirty solution for handling both local colored output and CI plain output so that one can run the locally as well: https://docs.rs/strip-ansi-escapes/0.1.0/strip_ansi_escapes/

We can deal properly with colors in a future PR.

[greg-szabo]

CI config changes:
Previously, we had a chain_a and chain_b service set up and the realyer-cli called with the relayer_conf_example.toml configuration. This is the setup described in the main README of the repo. The test was just to start the relayer successfully.

This previous setup is not very useful because as it turns out gaiad (the implementation of chain_a and chain_b) is not ready for any kind of IBC testing.

This new setup comments out the currently unusable test. In the future (when gaiad is working again), we should do integration testing with two chains and this code will be relevant again. I can remove it to keep things clearer, but we'll definitely readd this in the future.

The new setup establishes the use of the simapp, which is the Cosmos-SDK testing tool. This tool has enough implementation details now to be useful for query testing. We cannot test transactions with it, though, so there's no point in settin g up two of it. Simapp has a different configuration (chain name, etc), so we use the appropriate simple_config.toml configuration with it - or in this testing case, the simd_config() function will set up the relayer to use the details of the simd setup.

In the future, the current simd-based testing should go away and the currently commented out chain_a/chain_b testing should return.

[greg-szabo]

I looked into using trip_ansi_escapes with the tests but I'm running into interesting stuff: Abscissa has it's internal method to assert the standard out. I give it the string it should be and it internally compares it to the stdout of the application. Which means I have no means of stripping the stdout of colors, if I want to use the internal expect_line or expect_regex methods.

I should be able to extract the stdout from the result and manually strip it and parse it. All I'm losing is the nice feature where I hand over a Vector of strings and it automatically parses items as lines of the stdout, but I should be able to reimplement that (or parse out the first line, which is all we need.)

I feel we're trying to apply a quick and dirty solution that guts out some of the Abscissa features which is the main point of using Abscissa testing. I'd rather hold on with this and implement the currently agreed method of leaving out Abscissa and command-line parsing from testing.
After that's implemented we can compare and contemplate what method would be better. Then, I'd be happy to build a color-neutral version of the parser or apply this strip method to the stdout.

How does that sound? If you feel strongly about applying trip_ansi_escapes, I'll work on it, but maybe we can gain more by postponing it to a later step.

[greg-szabo]

I've implemented the three tests without abscissa too. Based on my experience we should remove the abscissa tests, they are not needed and the new tests are more robust.

As a next step, I would like to decompose the "simd" service into some mocked chain, but I need some more information on that. I'm not sure if that should go into this PR or a separate one.

[romac]

How does that sound? If you feel strongly about applying strip_ansi_escapes, I'll work on it, but maybe we can gain more by postponing it to a later step.

Sure, that sounds good! I was just mentioning as a potential quick and dirty hack, but didn't realize it wouldn't even work in that case.

[romac]

This new setup comments out the currently unusable test. In the future (when gaiad is working again), we should do integration testing with two chains and this code will be relevant again. I can remove it to keep things clearer, but we'll definitely readd this in the future.

Thanks for the explanation! I am fine with leaving the commented out config, but we should perhaps add a comment there explaining why it's commented out.

[greg-szabo]

Good point about the CI. I looked through it again and realized that it's not worth keeping it.

In this last commit, I

• Removed CI commented out code
• Removed Abscissa query tests - they can't handle color/non-color output in a seamless fashion
• Moved the latest query tests into an integration.rs for better code arrangement

[romac]

Looks great! Much more readable, but also much more robust since we are not testing against the output of the CLI, which will likely change substantially in the future. Great job :)