cpydiff: Fix test case for json_nonserializable. #16140
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #16140 +/- ##
=======================================
Coverage 98.57% 98.57%
=======================================
Files 164 164
Lines 21345 21345
=======================================
+ Hits 21040 21041 +1
+ Misses 305 304 -1 ☔ View full report in Codecov by Sentry. |
jepler
force-pushed
the
fix-json-difference-test
branch
from
7226a7b to
fc729b9
Compare
dpgeorge
reviewed
Nov 4, 2024
| x = json.loads(z) | ||
| print("Should not get here") | ||
| z = json.dumps(json) | ||
| print("Produced non-conforming json string", repr(z)) |
Member
There was a problem hiding this comment.
I think the original test was testing that bytes are not serialisable, which would be more of a subtle difference between CPython and MicroPython than trying to serialise a module.
Suggest to simply do:
try:
print(json.dumps(b"shouldn't be able to serialise this")
except TypeError:
print("TypeError")
Contributor
Author
|
In documentation, the existing test gives a UnicodeError. That's why I decided to try a whole different kind of object. I'll change the test case as you suggest. |
The test case was producing the following error: ``` Traceback (most recent call last): File "<stdin>", line 12, in <module> UnicodeError: ``` which did not demonstrate the intended difference. (this particular non-json-serializable object DID throw an exception! just not TypeError) The updated test uses a byte string with all ASCII bytes inside, which better illustrates the diference. Signed-off-by: Jeff Epler <jepler@gmail.com>
jepler
force-pushed
the
fix-json-difference-test
branch
from
fc729b9 to
5016780
Compare
Contributor
Author
|
Updated... |
Contributor
Author
|
There are CI errors but they appear unrelated to this change. -- /home/runner/work/micropython/micropython/tests/results/thread_thread_lock4.py.exp 2024-11-07 16:44:53.393277488 +0000
+++ /home/runner/work/micropython/micropython/tests/results/thread_thread_lock4.py.out 2024-11-07 16:44:53.393277488 +0000
@@ -58,3 +58,4 @@
77 145183092028285869634070784086308284983740379224208358846781574688061991349156420080065207861248000000000000000000
78 11324281178206297831457521158732046228731749579488251990048962825668835325234200766245086213177344000000000000000000
79 894618213078297528685144171539831652069808216779571907213868063227837990693501860533361810841010176000000000000000000
+CRASH
\ No newline at end of file
FAILURE /home/runner/work/micropython/micropython/tests/results/thread_thread_lock4.py--- /home/runner/work/micropython/micropython/tests/results/extmod_time_time_ns.py.exp 2024-11-07 16:44:35.601734727 +0000
+++ /home/runner/work/micropython/micropython/tests/results/extmod_time_time_ns.py.out 2024-11-07 16:44:35.601734727 +0000
@@ -4,7 +4,7 @@
True
True
True
-True
+False
True
True
True
FAILURE /home/runner/work/micropython/micropython/tests/results/extmod_time_time_ns.py |
Member
|
I reran the failing CI and now it passes. |
Member
|
Rebased and merged in 3844733 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The test case was producing the following error:
which did not demonstrate the intended difference. (this particular non-json-serializable object DID throw an exception! just not TypeError)
The updated version creates the non-conforming json string
"<module 'json'>"instead, better illustrating the problem.I tested by manually running the example using a coverage build & in cpython.