Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

tests: add first Unit tests #104

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

techno-disaster
Copy link
Contributor

@techno-disaster techno-disaster commented Jun 21, 2020

Description

Units tests for flutter client :) . Not all atm tho, these tests are only for auth and login. Also added flutter test in the gitthub ci/cd to have unit tests on each PR and build. Finally this also integreate codecov.io with which we can keep a check on the current code coverage as shown below.

image

Fixes #106

Flutter Channel:

  • I have used the Flutter Beta channel on my local machine

Type of Change:

Delete irrelevant options.

  • Quality Assurance

Code/Quality Assurance Only

  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Physical device, flutter test screenshot below.

image

We also get a really good coverage for the part thats written, code conv screenshot below
image

Checklist:

Delete irrelevant options.

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged

Code/Quality Assurance Only

  • My changes generate no new warnings

@techno-disaster techno-disaster added Category: Quality Assurance Changes to code or files that improve testing or fixes bugs. Status: Needs Review PR needs an additional review or a maintainer's review. Type: Testing UI Tests, Integration Tests, Travis CI, etc. labels Jun 21, 2020
@techno-disaster
Copy link
Contributor Author

@anitab-org/qa-team Can someone test this? Nothing should be broken but just to be safe :) Only login had some changes, so if that works everything else will work like before .

robotjellyzone
robotjellyzone previously approved these changes Jun 21, 2020
Copy link

@robotjellyzone robotjellyzone left a comment

Choose a reason for hiding this comment

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

@techno-disaster All the tests are running as can be seen in the following screenshots 👍

tests

tests2

tests3

and login functionality is also working fine :

login_tested

@robotjellyzone robotjellyzone removed the Type: Testing UI Tests, Integration Tests, Travis CI, etc. label Jun 21, 2020
@techno-disaster
Copy link
Contributor Author

shouldn't have pushed coverage 🤦

@techno-disaster
Copy link
Contributor Author

The current android test fails because we do not have the secret setup, will contact May on Zulip regarding this.

@techno-disaster
Copy link
Contributor Author

Codecov Report

exclamation No coverage uploaded for pull request base (develop@48318c4). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #104   +/-   ##
==========================================
  Coverage           ?   18.02%           
==========================================
  Files              ?       25           
  Lines              ?      283           
  Branches           ?        0           
==========================================
  Hits               ?       51           
  Misses             ?      232           
  Partials           ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48318c4...cd953c5. Read the comment docs.

uwu :)
base fix error will go once we merge atleast one codecov change to develop. @meenakshi-dhanani is this what you were suggesting on the product roadmap session?

@techno-disaster
Copy link
Contributor Author

techno-disaster commented Jun 25, 2020

image

Readme badge looks good too. Looks like travis is setup :) Will send final commit soon

bartekpacia
bartekpacia previously approved these changes Jul 20, 2020
@techno-disaster
Copy link
Contributor Author

Anyways i was able to fix this 🤦 wasn't thinking straight earlier i guess. Could you give it a quick review @bartekpacia @isabelcosta

@isabelcosta
Copy link
Member

Are you doing the "run tests" step on GitHub Actions / TravisCI? @techno-disaster

@techno-disaster
Copy link
Contributor Author

techno-disaster commented Jul 20, 2020

Are you doing the "run tests" step on GitHub Actions / TravisCI? @techno-disaster

it's on travis @isabelcosta I know we have planned to move to GH actions, but can wr merge this first and move the CI/Cd in a different issue and PR?

isabelcosta
isabelcosta previously approved these changes Jul 20, 2020
@techno-disaster techno-disaster added Status: Needs Review PR needs an additional review or a maintainer's review. and removed Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. labels Jul 22, 2020
@techno-disaster
Copy link
Contributor Author

@bartekpacia could you review this?

@techno-disaster
Copy link
Contributor Author

@anitab-org/mentorship-flutter-maintainers any update on this? I'm planning to write more tests today and send a PR, would like to see the new codecov bot working on it.

@isabelcosta
Copy link
Member

I already approved @techno-disaster
Can you start creating the issues for each test you are thinking of creating, and leave some of them open for the community to take and use your tests as an example. 1 issue per feature you wish to test would be the best, so PRs as smaller and easier to review.

@techno-disaster
Copy link
Contributor Author

Status on hold, waiting for #117 to merge because bloc 6.0.1 has some breaking changes on bloc_test too. Once that gets merged I'll update this PR with bloc 6.0.1 too :)

@techno-disaster techno-disaster added the Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. label Jul 27, 2020
@isabelcosta
Copy link
Member

@techno-disaster can you fix the merge conflicts and then I can approve and call out for more reviewers to get this to the finish line

@techno-disaster
Copy link
Contributor Author

@techno-disaster can you fix the merge conflicts and then I can approve and call out for more reviewers to get this to the finish line

done

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (develop@a4d58f2). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop     #104   +/-   ##
==========================================
  Coverage           ?   17.25%           
==========================================
  Files              ?       25           
  Lines              ?      284           
  Branches           ?        0           
==========================================
  Hits               ?       49           
  Misses             ?      235           
  Partials           ?        0           

@isabelcosta
Copy link
Member

@techno-disaster where are you running these tests, in what github action? I don't see it.

@techno-disaster
Copy link
Contributor Author

@techno-disaster where are you running these tests, in what github action? I don't see it.

No, this is done on travis

@techno-disaster techno-disaster removed the Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. label Sep 10, 2020
@isabelcosta
Copy link
Member

@techno-disaster where are you running these tests, in what github action? I don't see it.

No, this is done on travis

got it, found it

image

Copy link
Member

@isabelcosta isabelcosta left a comment

Choose a reason for hiding this comment

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

Seems fine to me, but we need more reviews here, as I am not very well versed with Flutter. I also notice some inconsistencies in the folder names in the tests folder. But its a minor issue perhaps for another issue.

Great to see test coverage increasing in this project :) 👏 @techno-disaster

@isabelcosta
Copy link
Member

@robotjellyzone can you please add a review to this again?
We need one more to merge :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Category: Quality Assurance Changes to code or files that improve testing or fixes bugs. Status: Needs Review PR needs an additional review or a maintainer's review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit tests and Coverage
6 participants