Skip to content

Fixing non-deterministic dict. failure in extra_types_test.#11

Merged
craigcitro merged 1 commit intogoogle:masterfrom
dhermes:fix-json-string-order
Mar 31, 2015
Merged

Fixing non-deterministic dict. failure in extra_types_test.#11
craigcitro merged 1 commit intogoogle:masterfrom
dhermes:fix-json-string-order

Conversation

@dhermes
Copy link
Copy Markdown
Contributor

@dhermes dhermes commented Mar 31, 2015

Failure occurs when comparing encoded strings via json.dumps and apitools.base.py.encoding.MessageToJson.

The MessageToJson function has non-deterministic results, as does json.dumps.

@craigcitro This fixes the failure in #9.

Failure occurs when comparing encoded strings via json.dumps
and apitools.base.py.encoding.MessageToJson.

The MessageToJson function has non-deterministic results, as
does json.dumps.
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 39.85% when pulling b3947f2 on dhermes:fix-json-string-order into c431900 on google:master.

1 similar comment
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 39.85% when pulling b3947f2 on dhermes:fix-json-string-order into c431900 on google:master.

craigcitro added a commit that referenced this pull request Mar 31, 2015
Fixing non-deterministic dict. failure in extra_types_test.
@craigcitro craigcitro merged commit 74c7af5 into google:master Mar 31, 2015
@dhermes dhermes deleted the fix-json-string-order branch March 31, 2015 21:08
@dhermes
Copy link
Copy Markdown
Contributor Author

dhermes commented Mar 31, 2015

@craigcitro Are you not concerned that encoding.MessageToJson has non-deterministic behavior?

@craigcitro
Copy link
Copy Markdown
Contributor

it's something to clean up, but we only really use JSON encoding for sending things over the wire (and the server doesn't care about key order). unless i'm missing something, this should only be an issue for tests that use literal json strings, right?

i was planning to file an issue later, but i'll do it now.

@dhermes
Copy link
Copy Markdown
Contributor Author

dhermes commented Mar 31, 2015

Yeah I wasn't sure it mattered, but figured it might because this test failure.

@craigcitro
Copy link
Copy Markdown
Contributor

i have a record/replay framework on top of apitools internally that i'm planning to push out at some point; that would likely be more fragile on this front. ;)

@dhermes
Copy link
Copy Markdown
Contributor Author

dhermes commented Mar 31, 2015

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants