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
Random Failures during tox run on Mac #1398
Comments
This happens on my system too, exactly the same error - and I use Arch Linux, so it's not an OS problem 😒 |
@vltr Well, good to know that I am not the only one who runs into this issue. Btw, can we tweak the test that checks the |
Yeah, these tests are pretty flakey. @ashleysommer may have more context. |
Is there any way to reliably make them fail? I've run them a few times on my main PC (Ubuntu 18.10), my work PC (Ubuntu 18.04) and my Laptop (Fedora 27), but I can't get them to fail at all. |
@ashleysommer But this happens only in case of |
Ah, I only have python 3.6 (not 3.7) so my system doesn't run the py37 tests. |
These tests are also flakey with Python3.6 on MacOS |
@vltr @ashleysommer @yunstanford Pardon my silly question, but shouldn't the content length value be same irrespective of what platform you are running in? |
I confirm the reports from @harshanarayana regarding The problem is: under >>> import json
>>> import ujson
>>> json.dumps({"hello": "world", "foo": "bar"})
'{"hello": "world", "foo": "bar"}'
>>> ujson.dumps({"hello": "world", "foo": "bar"})
'{"hello":"world","foo":"bar"}' To fix that, you just need to tell the >>> json.dumps({"hello": "world", "foo": "bar"}, separators=(',', ':'))
'{"hello":"world","foo":"bar"}' I think a minor PR should fix that in responses.py using from json import dumps as _json_dumps
json_dumps = partial(_json_dumps, separators=(',', ':')) |
@vltr Creating a PR with modified Test cases in a while. Saved me the effort of debugging the issue. :) |
@harshanarayana great! I'm glad to help, I'm just sorry I don't have the proper time right now to provide this PR myself ... |
The current implementation of `sanic` attempts to make use of `ujson` if it's available in the system and if not, it will default to the inbuilt `json` module provided by python. The current implementation of `ujson` does not provide a mechanism to provide a custom `seperators` parameter as part of the `dumps` method invocation and the default behavior of the module is to strip all the spaces around seperators such as `:` and `,`. This leads to an inconsistency in the response length when the response is generated using the `ujson` and in built `json` module provided by python. To maintain the consistency, this commit overrides the default behavior of the `dumps` method provided by the `json` module to add a `seperators` argument that will strip the white spaces around these character like the default behavior of `ujson` This addresses the issue referenced in sanic-org#1398 Signed-off-by: Harsha Narayana <harsha2k4@gmail.com>
py35: commands succeeded
py36: commands succeeded
py37: commands succeeded
py35-no-ext: commands succeeded
py36-no-ext: commands succeeded
py37-no-ext: commands succeeded
lint: commands succeeded
check: commands succeeded
congratulations :) Looks so good 😀 |
Oh yeah! 🤘 |
@vltr @ashleysommer Found another odd behavior with the tox env setup. setenv =
{py35,py36,py37}-no-ext: SANIC_NO_UJSON=1
{py35,py36,py37}-no-ext: SANIC_NO_UVLOOP=1 However, the ➜ .tox/py37-no-ext/bin/python
Python 3.7.0 (default, Aug 22 2018, 15:22:33)
[Clang 9.1.0 (clang-902.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ujson
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'ujson'
>>>
➜ .tox/py36-no-ext/bin/python
Python 3.6.7 |Anaconda, Inc.| (default, Oct 23 2018, 14:01:38)
[GCC 4.2.1 Compatible Clang 4.0.1 (tags/RELEASE_401/final)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ujson
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'ujson'
>>>
➜ .tox/py35-no-ext/bin/python
Python 3.5.6 (default, Sep 29 2018, 20:32:14)
[GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ujson
>>> |
That's odd, but sort of explains why only Python 3.6 and 3.7 |
@vltr Yes, it does. Both |
@harshanarayana perhaps this part of |
@vltr Nope. It's working exactly the same way across all 3 |
@harshanarayana that's weird (and random). Tomorrow I'll take a look directly into some clean environments to check what may be happening with Python 3.5 ... |
@harshanarayana somehow, if I execute only the
|
@vltr I did the same thing with no luck. I even tried the |
@harshanarayana exactly. Something is not right and I'll find out soon 😉 |
@vltr Please do let me know if there is anything I can do to help your investigation into this criminal matter. 😉 |
Will do ... Because this is getting stranger and stranger ... |
@vltr when in doubt.... blame it on the gremlins. |
@ahopkins yeah, well, it's starting to rain in here, so ... More fun stuff (inside the Sanic repository using pip-tools):
|
I finally found the problem 😓
Even the full test is now reporting "correctly" (prior to PR #1403):
How? Dead simple. The problem is here. When installing This is way too random. Maybe because I tried to set
Anyway, here's
And here's
That was a needle in a hay. @yunstanford should we just remove |
@vltr Wow. That's some detailed analysis. Thanks for doing this. If we want to keep |
@harshanarayana Erm ... I might need to disagree. I think that no software should have a circular dependency in any stage, and I'm going to make a PR for that matter. As for |
Removing |
@vltr Now that you found the root cause for the problem, let me see what we can do to fix the problem as well. I'm sure we can come up with something. |
@harshanarayana don't worry, I already addressed the issue with the core-devs to find the best solution - IMO |
@vltr If we remove So, now that we are only going to use the |
@vltr Great. Thank you very much for the help. 🏆 |
@harshanarayana it is I that should thank you for bringing this up, it really unfolded a very tricky situation that needs to be solved 😉 |
|
But I don't have strong opinion here, i'm fine to remove |
@yunstanford I know Since we are talking about this, why not bring |
removing I'm fine with moving the repo when we come up a proposal for bringing up all Sanic related projects together and also how we maintains them. |
@yunstanford great, we can sleep on that and think about proposals for the future 😉 |
@yunstanford , thanks for "fixing" pytest-sanic regarding this issue. I'll bring up the matter for bringing some Sanic related projects (that are really a plus to have, like yours) into the |
@sjsadowski @yunstanford I think we can close this issue by now. |
@vltr I can confirm that these random intermittent errors are seen no more. We can close this issue (The original issue I reported are all sorted). Thanks, everyone for the help :) |
Describe the bug
When running Unit Test in local machine (mac), few test cases randomly fail and re-cover during re-run.
Error Output
Code snippet
NA
Expected behavior
Existing Unit Tests to pass in all Virtual env during tox execution.
Environment (please complete the following information):
Additional context
NA
The text was updated successfully, but these errors were encountered: