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

Added CL/CV Support and tests #19

Merged
merged 27 commits into from Jan 28, 2020
Merged

Added CL/CV Support and tests #19

merged 27 commits into from Jan 28, 2020

Conversation

@flabbet
Copy link
Contributor

flabbet commented Jan 22, 2020

I added a CL/CV (Travis CL, code Codecov) support and one test, that passes.

EDIT: Set up test enviroment with all enviroment variables. I am currently writing more tests

@pep8speaks

This comment has been minimized.

Copy link

pep8speaks commented Jan 22, 2020

Hello @flabbet! Thanks for updating this PR.

Line 56:66: E126 continuation line over-indented for hanging indent

Comment last updated at 2020-01-27 20:25:34 UTC
flabbet added 5 commits Jan 22, 2020
Added pip3 install codecov
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 22, 2020

Codecov Report

❗️ No coverage uploaded for pull request base (master@081a7b6). Click here to learn what that means.
The diff coverage is 41.46%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #19   +/-   ##
=========================================
  Coverage          ?   55.39%           
=========================================
  Files             ?        3           
  Lines             ?      343           
  Branches          ?       46           
=========================================
  Hits              ?      190           
  Misses            ?      146           
  Partials          ?        7
Impacted Files Coverage Δ
swaglyrics_backend/utils.py 49.25% <33.33%> (ø)
swaglyrics_backend/issue_maker.py 56.72% <44.82%> (ø)

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 081a7b6...7c82795. Read the comment docs.

flabbet added 5 commits Jan 22, 2020
@flabbet

This comment has been minimized.

Copy link
Contributor Author

flabbet commented Jan 22, 2020

Wrote 5 tests. Tomorrow I'll write some more

@flabbet

This comment has been minimized.

Copy link
Contributor Author

flabbet commented Jan 24, 2020

I covered most of main functions, except ones that use @request_from_github() decorator. I am just not successful with patching this. But I still think this is a good start

@flabbet flabbet requested a review from aadibajpai Jan 24, 2020
@@ -50,10 +50,10 @@

SQLALCHEMY_DATABASE_URI = "mysql+mysqlconnector://{username}:{password}@{username}.mysql.pythonanywhere-services." \
"com/{username}${databasename}".format(
username=username,

This comment has been minimized.

Copy link
@aadibajpai

aadibajpai Jan 25, 2020

Member

revert this maybe? formatting changes interfere with reviewing the main code

This comment has been minimized.

Copy link
@aadibajpai

aadibajpai Jan 25, 2020

Member

It's still slightly different :P

tests/test_issue_maker.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@flabbet flabbet requested a review from aadibajpai Jan 25, 2020
def test_that_title_mismatches(self):
from swaglyrics_backend.issue_maker import is_title_mismatched
self.assertTrue(
is_title_mismatched({"Bohemian", "Rhapsody", "by", "Queen"}, "Miracle by Caravan Palace", 2))

This comment has been minimized.

Copy link
@aadibajpai

aadibajpai Jan 25, 2020

Member

Why do you pass a set instead of a normal string?

This comment has been minimized.

Copy link
@flabbet

flabbet Jan 27, 2020

Author Contributor

check is_title_mismatched function, it accepts words as first parameter and full title as second

This comment has been minimized.

Copy link
@aadibajpai

aadibajpai Jan 27, 2020

Member

Right so actually we call split() which returns a list. The thing with set is that every element should be unique which may not exist for some songs. You should use a list here to mimic as close to the actual functioning

This comment has been minimized.

Copy link
@aadibajpai

aadibajpai Jan 27, 2020

Member

words = title.split()

This comment has been minimized.

Copy link
@flabbet

flabbet Jan 27, 2020

Author Contributor

Didn't know that set must have unique values, I'll change that

This comment has been minimized.

Copy link
@aadibajpai
@flabbet

This comment has been minimized.

Copy link
Contributor Author

flabbet commented Jan 27, 2020

Fixed merge conflict. But somehow codecov broke. Logs look normal, idk why. I don't think it's my fault.

EDIT: It fixed itself somehow

@aadibajpai aadibajpai merged commit 729a0d6 into SwagLyrics:master Jan 28, 2020
3 checks passed
3 checks passed
Travis CI - Pull Request Build Passed
Details
codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.