Skip to content

tests/extmod/ussl_basic.py fails for modussl_mbedtls #4364

Description

@pfalcon

Well, subj. ussl_basic.py is coded too much relying on modussl_axtls behavior.

--- extmod_ussl_basic.py.exp	2018-12-16 22:45:07.539600906 +0300
+++ extmod_ussl_basic.py.out	2018-12-16 22:45:07.539600906 +0300
@@ -1,9 +1,7 @@
-ssl_handshake_status: -256
+mbedtls_ssl_handshake error: -7280
 wrap_socket: OSError(5,)
-<_SSLSocket 
-setblocking: NotImplementedError
-4
-b''
-read: OSError(-261,)
-read: OSError(9,)
-write: OSError(9,)
+mbedtls_ssl_handshake error: -7280
+Traceback (most recent call last):
+  File "extmod/ussl_basic.py", line 18, in <module>
+OSError: [Errno 5] EIO
+CRASH

Activity

  1. Jongy commented on Feb 7, 2019

    @Jongy
    Contributor

    Hmm.. They really expect an inherently different behavior.

    What should we do in such case? Is there any option to run tests based on external reading of mpconfig.h? If not, then perhaps we should add a ussl.backend resolving to axtls or mbedtls, then the test run only the relevant cases based on the implementation.

    Btw, this partly stems the same problem as I've described in #4475.

  2. pfalcon commented on Feb 7, 2019

    @pfalcon
    ContributorAuthor

    That test is just a hack.

    then perhaps we should add a ussl.backend

    Adding more bytes is not allowed ;-). (Well, in coverage build, everything is allowed).

  3. Jongy commented on Feb 8, 2019

    @Jongy
    Contributor

    In the CI, run-tests uses the coverage build? If so, I can add a MICROPY_PY_USSL_BACKEND thing, enabled only for coverage.

  4. pfalcon commented on Feb 8, 2019

    @pfalcon
    ContributorAuthor

    Well, as I told, that test is hack. Patching it with even more hacks unlikely seems like a good investment of effort.

    Instead imagine how a real test would be written: allow to override time and random sources for the underlying TLS lib, then feed predefined/read expected TLS records. Well, could also implement real TLS handling on the tester side ;-).

    If it doesn't do that, it's a hack. And if some lines of a hack doesn't work with different TLS implementations, those lines are very bad hack. Their removal is obvious solution. Well, could also keep them in a dirty attempt to keep coverage, just test for variants of expected behavior explicitly (not by printing stuff).

  5. pfalcon commented on Mar 1, 2019

    @pfalcon
    ContributorAuthor

    Note that #4481 was posted also as an example that adding any adhoc, mis-leveled tests (which test not TLS, but low-levelish, corner behavior of current implementations) would only complicate matters.

  6. added
    testsRelates to tests/ directory in source
    on Jul 16, 2022
  7. dpgeorge commented on Jul 19, 2022

    @dpgeorge
    Member

    Fixed by 18ecc29

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    testsRelates to tests/ directory in source

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions