-
Notifications
You must be signed in to change notification settings - Fork 69
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
Introduce pytest to help address time issues to run journey tests #142
Conversation
7ace0be
to
43eee70
Compare
As usual government web API are down on weekends. Gonna re-run tests later (or on Monday)… |
@lipemorais Travis CI is still broken with an
|
I will do it soon. :)
Here is the url: http://www.camara.leg.br/cotas/Ano-2009.csv.zip @cuducos I will add t |
Travis is green, yay! I think all we need now is a version bump ; ) |
@@ -20,7 +20,7 @@ def setUp(self): | |||
def tearDown(self): | |||
rmtree(self.path, ignore_errors=True) | |||
|
|||
|
|||
@skip('Takes to long to run') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just something to test the CI or is it supposed to go to master
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a test, looks that this test were making build fail using all memory available on Travis.
9c3ff48
to
57d01c6
Compare
def test_fetch_translate_clean_integration(self): | ||
self.subject.fetch() | ||
files = ["Ano-{}.csv".format(n) for n in self.years] | ||
files = ["Ano-{}.csv".format(n) for n in [2017]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about self.years[-1:]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just a attempt to make this test run without use all the resources available in Travis CI and break the build because of it. Looks that worked because it's breaking for a different reason now.
There is a test breaking because this dataset is not available. :'( |
Hey! @cuducos @jtemporal @anaschwendler do you have any idea about what can I do to fix this? :( |
What about mocking the requests and using tiny fixtures instead? |
@cuducos this is what we do in unit version of this test. |
Yep. An IMHO opinion depending on external services to run the suites tests is a problem per se. If I'm not wrong we adopted the journey test this way (depending on external services) as a temporary fix. As this was not a permanent solution and, given that it is broken now, it might be an interesting opportunity to sor that out. Not sure about what others @anaschwendler @jtemporal @Irio think of that though… |
Hey, @cuducos @anaschwendler . Looks that it is working now. Could we re run travis? |
Sure thing! Just restarted it ; ) |
Hasn't lasted long enough:
|
Looks now that http://www.camara.leg.br/cotas/Ano-2008.csv.zip is out. :'( |
I just runned it a ALL tests and where all green. :) Could we rerun it or just merge? |
Yay! May we have a version bump? |
3ec5879
to
a7a2dc2
Compare
Done in a7a2dc2 |
What is the purpose of this Pull Request?
The purpose of this PR is use pytest to help address the long time running some journey tests usings
--durations
, this way we know which test takes more time to run.What was done to achieve this purpose?
I changed way we run tests to use pytest.
How to test if it really works?
To test it you can just run
pytest --verbose --durations=10 --cov-report html tests/unit
after install pytest and pytest-cov usingpip install pytest pytest-cov
.Who can help reviewing it?
@cuducos @jtemporal @anaschwendler
TODO
pytest.ini
fileYesterday I runned it on my machine and some tests took too long to run