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

R4R: Increasing test coverage - First Pass #3517

Merged
merged 15 commits into from
Feb 8, 2019

Conversation

jleni
Copy link
Member

@jleni jleni commented Feb 6, 2019

This PR includes a first group of additional tests. Other PRs will be submitted afterwards.

Related to #2004

  • Wrote tests
  • Updated relevant documentation (docs/)
  • Added entries in PENDING.md with issue #
  • rereviewed Files changed in the github PR explorer

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@jleni jleni changed the title WIP: Increasing test coverage WIP: Increasing test coverage - First Pass Feb 7, 2019
@jleni jleni force-pushed the tests/coverage branch 2 times, most recently from 0036a69 to 60b5658 Compare February 7, 2019 07:28
@jleni jleni changed the title WIP: Increasing test coverage - First Pass R4R: Increasing test coverage - First Pass Feb 7, 2019
@jleni jleni added T: Tests Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. ready-for-review labels Feb 7, 2019
Makefile Outdated Show resolved Hide resolved
client/context/context.go Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
client/context/broadcast.go Outdated Show resolved Hide resolved
client/keys/add.go Outdated Show resolved Hide resolved
client/keys/update.go Outdated Show resolved Hide resolved
Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

So far so good

@jackzampolin jackzampolin changed the title R4R: Increasing test coverage - First Pass WIP: Increasing test coverage - First Pass Feb 8, 2019
@jackzampolin
Copy link
Member

Moving this back to WIP

@jleni
Copy link
Member Author

jleni commented Feb 8, 2019

Moving this back to WIP

Yep. I will squash this a bit too, so I remove Alessio's already merged commits.

Copy link
Member

@jackzampolin jackzampolin left a comment

Choose a reason for hiding this comment

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

Great tests. Want to see a follow up to this that adds stdout and stdin to context and applies that to the codebase.

@jleni
Copy link
Member Author

jleni commented Feb 8, 2019

Great tests. Want to see a follow up to this that adds stdout and stdin to context and applies that to the codebase.

Yep, the issue is here. #3558

@jleni
Copy link
Member Author

jleni commented Feb 8, 2019

grr.. circleci is not doing well today :(

@alessio
Copy link
Contributor

alessio commented Feb 8, 2019 via email

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

This looks great @jleni -- I just have a few remarks/questions.

client/input.go Show resolved Hide resolved
client/keys/add.go Outdated Show resolved Hide resolved
client/keys/delete.go Outdated Show resolved Hide resolved
client/keys/show.go Outdated Show resolved Hide resolved
client/keys/utils.go Show resolved Hide resolved
@jleni
Copy link
Member Author

jleni commented Feb 8, 2019

@alexanderbez ready 👍

@jackzampolin
Copy link
Member

@jleni still failing a test 🙃

jleni and others added 15 commits February 8, 2019 20:51
Squashed commits:
[72934fc7] Adding tests (WIP)
[0a342270] Adding additional tests
Squashed commits:
[e72a3b57] Additional tests
[643ca3f8] revert rename
[9100fe61] Add more tests, remove linter warnings
[23f092bb] more tests
Adjusting to new keybase handling
Better test + redirecting inputs
Updating delete test to use fake user input
Clean up linters
Add more tests, remove linter warnings

revert rename

Additional tests

fix linter issue
Co-Authored-By: jleni <[email protected]>
@jleni
Copy link
Member Author

jleni commented Feb 8, 2019

@jleni still failing a test 🙃

yes, reverting the "defensive" checks resulted in test failing :)

fixed and rebased.

@jackzampolin jackzampolin merged commit b5fdb83 into cosmos:develop Feb 8, 2019
@jleni jleni deleted the tests/coverage branch February 21, 2019 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Tests Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants