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

TVLA: Test histogram loading for general tests #95

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

andreaskurth
Copy link
Collaborator

@andreaskurth andreaskurth commented Jun 30, 2022

This tests the functionality implemented in #89.

@andreaskurth andreaskurth requested a review from vrozic June 30, 2022 10:53
@andreaskurth andreaskurth force-pushed the test-tvla_hist_load branch 2 times, most recently from b7c836e to 037938b Compare July 1, 2022 09:56
Signed-off-by: Andreas Kurth <adk@lowrisc.org>
@andreaskurth andreaskurth marked this pull request as ready for review July 1, 2022 10:03
test/tvla_test.py Outdated Show resolved Hide resolved
@abdullahvarici
Copy link

After we get some power measurements with capture.py script, we don't get the histogram files, right? I think we also need to add some other tests to cover the complete functionality of the tvla.py script (input may be hundreds of power traces and output may be the figures or their hashes). Other than that, it worked in my system and looks good to me.

In my PR, I have changed the commands in tvla_test.py to comply with the updated CLI of the tvla.py script: #92

This class represents the absolute path to a test data file.  This serves
two main purposes:
1. All test data files live in the same subdirectory, and this class
   encapsulates that commonality.
2. Canonically using an absolute path to access test data decouples the
   current working directory where a test is executed from the test result.

Signed-off-by: Andreas Kurth <adk@lowrisc.org>
@andreaskurth
Copy link
Collaborator Author

After we get some power measurements with capture.py script, we don't get the histogram files, right? I think we also need to add some other tests to cover the complete functionality of the tvla.py script (input may be hundreds of power traces and output may be the figures or their hashes).

Absolutely, this is just the first step in testing the full TVLA! Could you create an issue to collect aspects of TVLA that need testing, @abdullahvarici? We can then discuss and prioritize them together.

Other than that, it worked in my system and looks good to me.

Great, thanks!

In my PR, I have changed the commands in tvla_test.py to comply with the updated CLI of the tvla.py script: #92

Nice, let's focus on getting that merged as soon as this is approved and merged.

Copy link
Collaborator

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good!

I would find it useful to add the command to run the test (pytest test in the main directory) into the commit message.

test/tvla_test.py Outdated Show resolved Hide resolved
This adds two tests: each test takes a histogram as input, runs TVLA on
it, and checks whether the resulting t-test traces contains a
significant deviation from the mean, which would indicate leakage.  For
one histogram we expect leakage, for the other we do not.

These tests are automatically discovered by pytest.  Run `pytest test`
in the top-level directory to execute them.

Signed-off-by: Andreas Kurth <adk@lowrisc.org>
@andreaskurth andreaskurth merged commit 27add68 into lowRISC:master Jul 4, 2022
@andreaskurth andreaskurth deleted the test-tvla_hist_load branch July 4, 2022 10:07
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.

None yet

4 participants