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

test #21

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

test #21

wants to merge 3 commits into from

Conversation

yaruwangway
Copy link
Contributor

@yaruwangway yaruwangway commented Jul 8, 2021

  • tested confirmuser, brrr, tilbrrr, send

  • changed TxErr. This function parses the "out" from command "pooltoy tx bank send", however, there is no output JSON flag available for this command?? therefore, this command-generated output string cannot be unmarshalled to a go struct. I use the map to store the result.

  • please add acounts.json file and test_variable.go file to this repo when test.
    example of test_variable.go:
    var ( UserID1 = "xxxxyyyy" UserName1 = "lisa" User1 ="<@xxxxyyyy|lisa")

@yaruwangway yaruwangway requested a review from okwme July 8, 2021 17:57
@yaruwangway yaruwangway linked an issue Jul 8, 2021 that may be closed by this pull request
18 tasks
main.go Outdated
@@ -546,6 +612,9 @@ func balance(userid string, text []string) string {
}

func main() {

go testserver.ForwardServer()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still needed?

@okwme
Copy link
Contributor

okwme commented Jul 9, 2021

looks good!
nice work finding a solution to parsing the json. I think you could actually get back to the original behaviour by adding the --log_format string to the query like pooltoy q tx <txhash> --log_format json. I think the CLI updated so that plain text response is the default whereas before it was json:

      --log_format string   The logging format (json|plain) (default "plain")

It would also be good to see a bit more of the edge cases tested. For instance what happens if brr is called twice in a row? This should confirm that the error does occur. I think it makes sense to include the accounts.json in the repo if it's required for testing.

It would be good to figure out a way to test the confirmUser command to make sure that alice is able to create new users if they don't exist already.

@yaruwangway
Copy link
Contributor Author

yaruwangway commented Jul 9, 2021

about commit 199f573:

  • the parsing string to a map is still kept as a temporal solution till the cosmos-sdk --log_format flag works again.
  • edge cases test is added.
  • confirmUser function is changed:
    before change, the confirmUser function will call createNewUserKey() if shellout pooltoy keys show %s --keyring-backend test throws an error. The confirmUser() will return the error/nil from createNewUserKey() rather than the shellout. This is misleading: when a user does not exist, the confirmUser might not return an error if it createNewUserKey() successfully.
  • createNewUserAccount does not pass tests now, I will check further.

@yaruwangway
Copy link
Contributor Author

looks good!
nice work finding a solution to parsing the json. I think you could actually get back to the original behaviour by adding the --log_format string to the query like pooltoy q tx <txhash> --log_format json. I think the CLI updated so that plain text response is the default whereas before it was json:

      --log_format string   The logging format (json|plain) (default "plain")

It would also be good to see a bit more of the edge cases tested. For instance what happens if brr is called twice in a row? This should confirm that the error does occur. I think it makes sense to include the accounts.json in the repo if it's required for testing.

It would be good to figure out a way to test the confirmUser command to make sure that alice is able to create new users if they don't exist already.

About including accounts.json, I am thinking to test in another way, let Alice create the users for test, and delete them afterwards. Then the init.sh and pooltoy start are only called once during all the tests, also, no need to take care of accounts.json, the user data is within the test function, anyone can pull and test without accounts.json. Would this be a good test method since everytime Alice has to create some users ?

@yaruwangway
Copy link
Contributor Author

regard to the commit 04c098d:

  1. key info is saved in the slackbot/key folder
  2. "insufficient funds" is returned directly after the transaction by adding -b block when doing transactions
  3. because of adding-b block, the transaction ouput structure is changed, therefore, re-parsed. (temporal solution: parseTxResultTxHash, parseTxResultCode)

@yaruwangway yaruwangway linked an issue Jul 15, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

record user key is broken Yaru Todos
2 participants