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

Unit tests for the utils available. #10

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Unit tests for the utils available. #10

wants to merge 6 commits into from

Conversation

esirK
Copy link
Contributor

@esirK esirK commented May 8, 2020

Description

Adds unittests for both gsheet helper and twitter stream listener.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

Screenshot 2020-05-08 at 06 58 23

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@esirK esirK marked this pull request as draft May 8, 2020 04:14
Copy link
Member

@kilemensi kilemensi left a comment

Choose a reason for hiding this comment

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

👍 @esirK

Quick review:
I'm all for unit tests but I'm more interested in integration tests & I believe those give us more bang for the buck. So rather than testing if GoogleSheetHelper has credentials set & what format those credentials are in, a more valuable test is can GoogleSheetHelper connect when given valid credentials? etc.

@kilemensi
Copy link
Member

PS: If a PR is under draft @esirK, the assumption is you're still working on it privately & you don't need reviews from the team just yet 😎 .

@esirK
Copy link
Contributor Author

esirK commented May 8, 2020

PS: If a PR is under draft @esirK, the assumption is you're still working on it privately & you don't need reviews from the team just yet 😎 .

I had talked to @ascii-dev on writing unit tests but after writing them, I wanted to discuss with the rest of the team on whether we should do integration tests instead of just unit tests that's why I made the PR a draft.

@ascii-dev ascii-dev added this to the v0.1 milestone May 20, 2020
@ascii-dev ascii-dev removed this from the v0.1 milestone Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants