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

tests script for board #28

Merged

Conversation

potsrevennil
Copy link
Contributor

@potsrevennil potsrevennil commented Apr 18, 2024

Mainly added a python script for running tests on board. Merged the original tests script into it as well.
Currently it's slow to run the nistkat test on board, due to the bottleneck of IO. Checking the checksum on board would be better.

☺  tests --help                                                                              
Usage: tests [OPTIONS] COMMAND [ARGS]...

Options:
  --help  Show this message and exit.

Commands:
  func     Run functional tests
  nistkat  Run nistkat tests
  run      Run for the specified platform and hex file without parsing the
           output
  speed    Run speed tests
  stack    Run stack tests
☺  tests func --help
Usage: tests func [OPTIONS]

Options:
  -cfg, --platform-cfg PATH  Configuration file of the specified platform
                             [default: hal/stm32f4discovery.cfg]
  -v, --verbose              Show verbose output or not
  -e, --emulate              Emulate on the QEMU or not
  -t, --ntests INTEGER       Number of tests  [default: 1]
  --help                     Show this message and exit.
  • add marker for tests when running on board
  • add python script for running tests on board
  • use the python tests script in ci instead
  • clean up the original ci test script
  • Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)

@potsrevennil potsrevennil marked this pull request as ready for review April 24, 2024 06:33
@potsrevennil potsrevennil requested a review from a team as a code owner April 24, 2024 06:33
Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, thank you!
I guess we want to make this the preferred way to perform tests and benchmarks. Can you please adjust the main README accordingly?

$ tests speed -t 100 -v
Traceback (most recent call last):
  File "/tmp/mlkem-c-embedded/scripts/tests", line 290, in <module>
    cli()
  File "/nix/store/r725a3hx83v9k6222xc6kamf8rmzzlxs-python3.11-click-8.1.7/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/r725a3hx83v9k6222xc6kamf8rmzzlxs-python3.11-click-8.1.7/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/nix/store/r725a3hx83v9k6222xc6kamf8rmzzlxs-python3.11-click-8.1.7/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/r725a3hx83v9k6222xc6kamf8rmzzlxs-python3.11-click-8.1.7/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/r725a3hx83v9k6222xc6kamf8rmzzlxs-python3.11-click-8.1.7/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/mlkem-c-embedded/scripts/tests", line 214, in speed
    base_test(
  File "/tmp/mlkem-c-embedded/scripts/tests", line 126, in base_test
    check(file, expect, actual)
  File "/tmp/mlkem-c-embedded/scripts/tests", line 107, in check
    sys.exit(1)
    ^^^
NameError: name 'sys' is not defined

test/speed.c Outdated Show resolved Hide resolved
scripts/tests Outdated Show resolved Hide resolved
@potsrevennil
Copy link
Contributor Author

Updated the README, but not sure if it's clear enough...

Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now. Thanks @potsrevennil!

@potsrevennil potsrevennil merged commit 544e38a into pq-code-package:main Apr 29, 2024
3 checks passed
@potsrevennil potsrevennil deleted the tests-script-for-board branch April 29, 2024 09:54
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.

2 participants