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

Simplify testing with dummy arithmetic #327

Closed

Conversation

markspanbroek
Copy link
Contributor

This introduces a DummyArithmeticRunner. It has a static run() method to run an application with dummy arithmetic.

We use this to remove about 200 lines of boilerplate code from the LinearAlgebraTests.

Can be used to simplify unit tests; no need to create a subclass of
TestThreadFactory for each test.
Incidentally fixes that some tests were not running because they were
missing from the TestDummyArithmeticProtocolSuite.
@markspanbroek markspanbroek mentioned this pull request Jan 9, 2019
@markspanbroek
Copy link
Contributor Author

Closing and opening to trigger a rebuild in Travis.

@codecov
Copy link

codecov bot commented Jan 10, 2019

Codecov Report

Merging #327 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #327    +/-   ##
========================================
  Coverage       100%   100%            
- Complexity     3312   3440   +128     
========================================
  Files           369    373     +4     
  Lines          9178   9626   +448     
  Branches        689    752    +63     
========================================
+ Hits           9178   9626   +448
Impacted Files Coverage Δ Complexity Δ
...alexandra/fresco/tools/mascot/field/InputMask.java 100% <0%> (ø) 4% <0%> (ø) ⬇️
...andra/fresco/suite/spdz2k/Spdz2kProtocolSuite.java 100% <0%> (ø) 4% <0%> (ø) ⬇️
...rc/main/java/dk/alexandra/fresco/demo/AesDemo.java 100% <0%> (ø) 17% <0%> (ø) ⬇️
...co/suite/dummy/bool/DummyBooleanProtocolSuite.java 100% <0%> (ø) 3% <0%> (ø) ⬇️
...nytables/prepro/TinyTablesPreproProtocolSuite.java 100% <0%> (ø) 3% <0%> (ø) ⬇️
...ra/fresco/suite/spdz/SpdzRoundSynchronization.java 100% <0%> (ø) 13% <0%> (ø) ⬇️
...fresco/lib/math/integer/division/KnownDivisor.java 100% <0%> (ø) 5% <0%> (+2%) ⬆️
...fresco/suite/spdz2k/datatypes/CompUIntFactory.java 100% <0%> (ø) 4% <0%> (-3%) ⬇️
...e/dummy/arithmetic/DummyArithmeticAddProtocol.java 100% <0%> (ø) 3% <0%> (ø) ⬇️
...ra/fresco/tools/mascot/MascotResourcePoolImpl.java 100% <0%> (ø) 14% <0%> (+1%) ⬆️
... and 100 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4ed21f...a78ac74. Read the comment docs.

Copy link
Collaborator

@pffrandsen pffrandsen left a comment

Choose a reason for hiding this comment

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

Great improvement. We should consider how to reuse tests across protocols, it is not clear to me that this is achievable with this PR

return run(application, 1);
}

static public <T> T run(Application<T, ProtocolBuilderNumeric> application, int numberOfParties) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion. Change run to:
run(
Supplier input,
Function<InputT, Application test,
Consumer verifier
)

This will make the tests stepwise present lambdas with different purposes

@GuutBoy
Copy link
Contributor

GuutBoy commented Jan 18, 2019

I like this simplification, and I am thinking it can go towards #198. However, you also loose some generality. The focus on the old way of writing the tests (as in the old LinearAlgebraTests) was that you could use the same test code for essentially any protocol suite (dummy/spdz/spdz2k and so on). In the suggested change, you have tied LinearAlgebraTest to the DummyArithmetic suite.

Would there be some way to achieve the same generality with this simpler method?

@markspanbroek
Copy link
Contributor Author

markspanbroek commented Jan 21, 2019

Thanks for mentioning the reuse of test code, @pffrandsen & @GuutBoy. Personally, I would be inclined to separate out those tests that test computations (such as the linear algebra ones) from the ones that test protocol suites (such as spdz). I would prefer to have tests fail for a single reason only.

However, if you really want to reuse test logic, then you can do this with the Runner concept as well. Most tests look like this:

void myTest() {
  // Define input
  // Create application
  run(application)
  // Check result
}

You could run them with different runners like this:

void myTestDummy() {
  app = createMyTestApp()
  result = DummyArithmeticRunner.run(app)
  checkMyTestResult(result)
}

void myTestSpdz() {
  app = createMyTestApp()
  result = SpdzRunner.run(app)
  checkMyTestResult(result)
}

Application<> createMyTestApp() {
  // Define input
  // Create application
}

void checkMyTestResult(result) {
  // Check result
}

@markspanbroek
Copy link
Contributor Author

Closing and opening to trigger a rebuild in Travis.

@markspanbroek
Copy link
Contributor Author

Closing due to inactivity.

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

3 participants