Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

[Refine #76] Avoid storing test data in repo. #84

Merged
merged 2 commits into from
Apr 17, 2016
Merged

[Refine #76] Avoid storing test data in repo. #84

merged 2 commits into from
Apr 17, 2016

Conversation

fkztw
Copy link
Member

@fkztw fkztw commented Apr 3, 2016

ref #76

@fkztw fkztw added the testing label Apr 3, 2016
fkztw pushed a commit that referenced this pull request Apr 3, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 66.891% when pulling eb10730 on issue-84 into 3f942ec on master.

word = 'Spanish'
timeout = 5
raw_html = d._get_raw(word, timeout)
record = d.query(word, timeout)
Copy link
Member

Choose a reason for hiding this comment

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

It seems that there is no function requires it but test_show.
How about just setup it as local variable of test_show?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, i found that test_query_normal require it.
Ignore previous comment.

Copy link
Member

Choose a reason for hiding this comment

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

I will suggest that pust line 9-13 in a setup/teardown method.
ref: https://pytest.org/latest/xunit_setup.html#method-and-function-level-setup-teardown
e.g.

class TestSpansisDict:
  def setup_method(self, method):
     self.d = SpanihDict()
      ...
  def teardown_method(self, method):
     del self.d
     del ...

This snippet can ensure our each testing method not to be effect by other else testing method. (or say isolation our testing method).
I think those vars (self.d, self.record ... etc) can consider as kind of fixture, so prepare them in setup methd will be resonable.

@fkztw
Copy link
Member Author

fkztw commented Apr 3, 2016

content in test_jisho.py also needs this refinement.

@fkztw
Copy link
Member Author

fkztw commented Apr 3, 2016

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 84.053% when pulling 4b2b9d0 on issue-84 into 48e9b78 on master.

fkztw pushed a commit that referenced this pull request Apr 17, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.0%) to 83.213% when pulling a3173cf on issue-84 into 48e9b78 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 84.151% when pulling f1fa990 on issue-84 into 48e9b78 on master.

@fkztw fkztw merged commit c10be62 into master Apr 17, 2016
@fkztw fkztw deleted the issue-84 branch April 4, 2017 04:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants