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

[RPC] Fix bug in Cryptol JSON -> Python for records, add test #1269

Merged
merged 3 commits into from
Aug 25, 2021

Conversation

m-yac
Copy link
Contributor

@m-yac m-yac commented Aug 24, 2021

This PR:

  • Fixes a bug which would result in incorrectly converting a record value from Cryptol JSON to Python.
  • Adds a new test test_types.py which would have detected this bug, and will detect any future changes to the way basic Cryptol types are handled.

While making this change I also noticed that there were some missing calls to .result() in test_AES.py and that test_DES.py had a typo. These two fixes are also included in this PR.

@m-yac m-yac requested review from atomb and pnwamk August 24, 2021 23:00
@m-yac
Copy link
Contributor Author

m-yac commented Aug 25, 2021

Should I be worried that the CI failed? The RPC tests all passed on ubuntu, but failed on macos with:

======================================================================
ERROR: test_CplxQ (test_CplxQNewtype.TestCplxQ)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/cryptol/cryptol/cryptol-remote-api/python/tests/cryptol/test_CplxQNewtype.py", line 13, in test_CplxQ
    forty_two = c.eval("fortyTwo").result()
  File "/Users/runner/Library/Caches/pypoetry/virtualenvs/cryptol-L3Aq5hhJ-py3.7/lib/python3.7/site-packages/argo_client/interaction.py", line 168, in result
    return self.process_result(self._result_and_state_and_out_err()[0])
  File "/Users/runner/Library/Caches/pypoetry/virtualenvs/cryptol-L3Aq5hhJ-py3.7/lib/python3.7/site-packages/argo_client/interaction.py", line 153, in _result_and_state_and_out_err
    raise self.process_error(exception)
argo_client.interaction.ArgoException: Unknown state ID 24bf76c8-1207-4b9c-8709-2e73caa435cd

This test works just fine on my (macos) machine.

@atomb
Copy link
Contributor

atomb commented Aug 25, 2021

That's a known bug that I think @pnwamk is working on fixing separately.

@m-yac m-yac merged commit b8bff7b into master Aug 25, 2021
@m-yac m-yac deleted the rpc/record-bug branch August 25, 2021 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants