Skip to content
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

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

Merged
merged 1 commit into from
Mar 31, 2015
Merged

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

merged 1 commit into from
Mar 31, 2015

Conversation

dhermes
Copy link
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

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

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
Contributor Author

dhermes commented Mar 31, 2015

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

@craigcitro
Copy link
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
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
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
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