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

tests: coverage increase #67

Merged
merged 2 commits into from
Jul 29, 2016
Merged

tests: coverage increase #67

merged 2 commits into from
Jul 29, 2016

Conversation

omelkonian
Copy link
Contributor

Signed-off-by: Orestis Melkonian melkon.or@gmail.com

@jirikuncar jirikuncar added this to the v1.0.0 milestone Jul 1, 2016
@@ -146,10 +146,10 @@ class RemoteToken(db.Model):
secret = db.Column(db.Text(), default='', nullable=False)
"""Used only by OAuth 1."""

def __repr__(self):
def __repr__(self): # pragma no cover
Copy link
Member

@jirikuncar jirikuncar Jul 1, 2016

Choose a reason for hiding this comment

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

Adding pragma no cover is not really the way to increase test coverage.

def test_token_repr(app):
    """..."""
    assert 'Remote Token ...' == RemoteToken(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, assumed it was good practice as I saw it as an example on the documentation of coverage.py (http://coverage.readthedocs.io/en/coverage-4.0.3/excluding.html). Anyway, updated.

token_type='type').__repr__()

assert 'Remote Account <id=1, user_id=1>' == \
RemoteAccount.get(user.id, "dev").__repr__()
Copy link
Member

Choose a reason for hiding this comment

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

repr(RemoteAccount.get(user.id, "dev"))

@jirikuncar
Copy link
Member

@omelkonian Good job❗ Can you please have a look at my last comments and let me know when you update the PR? Thanks

@omelkonian
Copy link
Contributor Author

@jirikuncar LGTY?

@jirikuncar
Copy link
Member

@omelkonian This branch is out-of-date with the base branch.

@jirikuncar
Copy link
Member

jirikuncar commented Jul 19, 2016

@omelkonian please remove the merge commit. (git rebase -i upstream/master)

@omelkonian
Copy link
Contributor Author

@jirikuncar i think its good to go

@jirikuncar
Copy link
Member

@omelkonian omelkonian changed the title tests: increase test coverage tests: coverage increase Jul 20, 2016
@jirikuncar
Copy link
Member

@omelkonian can you please rebase?

request_token_params={'scope': ''},
base_url='https://foo.bar/',
request_token_url=None,
access_token_url="https://foo.bar/oauth/access_token",
Copy link
Member

Choose a reason for hiding this comment

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

Can you please use consistently single quotes (')?

* Increases test coverage. (closes #59)

Signed-off-by: Orestis Melkonian <melkon.or@gmail.com>
base_app.register_blueprint(blueprint_settings)

# Try to sign-up client
base_app.test_client().get(url_for("invenio_oauthclient.signup",
Copy link
Member

Choose a reason for hiding this comment

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

Signed-off-by: Orestis Melkonian <melkon.or@gmail.com>
@omelkonian
Copy link
Contributor Author

Is it ok that I replaced string quotes to more places than I changed?

@jirikuncar
Copy link
Member

jirikuncar commented Jul 28, 2016

@omelkonian normally it would be better to do replacement in separate commit and then include your chances. Would you find a time to do it this way and then rebase your commit on top?

@omelkonian
Copy link
Contributor Author

@jirikuncar I put it on a separate commit

@lnielsen lnielsen merged commit 369bda4 into inveniosoftware:master Jul 29, 2016
@omelkonian omelkonian deleted the testing branch July 29, 2016 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: increase test coverage
3 participants