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

test: implement repository-wide unit tests #73

Merged
merged 16 commits into from
Jun 17, 2024
Merged

Conversation

brech1
Copy link
Collaborator

@brech1 brech1 commented Jun 14, 2024

Description

This PR implements unit testing for several modules. The majority of methods that didn't required a circuit mock were covered. Modules like runtime and compiler are almost entirely tested.

The untested methods could be tested through integration tests, or through some testing utils with mocks in a future PR.

The workflow actions have been updated to include a testing coverage report.

There has also been some codebase structure updates.

Two bugs have been fixed:

Declaration of constant signals in the compiler module

let node = Node::new_with_signal(id, value.is_some(), false);

The order of value.is_some() and false was switched. This was declaring every constant signal as an output signal, and not setting it as constant.

Signal substitution (found by @voltrevo)

Expression::Call { .. }
| Expression::InfixOp { .. }
| Expression::PrefixOp { .. }
| Expression::Number(_, _) => {
    // Get the signal identifiers and connect them
    let given_output_id = ctx.get_signal_id(&lh_access)?;
    let gate_output_id = get_signal_for_access(ac, ctx, signal_gen, &rh_access)?;

    ac.add_connection(gate_output_id, given_output_id)?;
}
_ => return Err(ProgramError::SignalSubstitutionNotImplemented),

If a signal was assigned to a value using a Call, PrefixOp or Number this was being ignored, so these values have been added to substitution case match. The program error SignalSubstitutionNotImplemented has also been implemented.

Changes

  • Implements new codecov report in CI.
  • Implements unit tests for multiple modules.
  • Ignores ./vscode settings.
  • Removes default bin config in cargo.toml.
  • Updates circuit input to be ./input/circuit.circom.
  • Removes examples folder and put those circuits in the test directory.
  • Removes ml-tests folder and put those circuits in the test directory.
  • Restructures integration tests module.
  • Adds IndexOutOfBounds integration test @curryrasul.

Related Issues

@brech1 brech1 marked this pull request as ready for review June 16, 2024 21:01
@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2024

Codecov Report

Attention: Patch coverage is 97.21254% with 24 lines in your changes missing coverage. Please review.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files Coverage Δ
src/arithmetic_circuit.rs 90.51% <ø> (ø)
src/cli.rs 90.00% <100.00%> (ø)
src/compiler.rs 77.44% <100.00%> (ø)
src/main.rs 0.00% <ø> (ø)
src/program.rs 97.67% <100.00%> (ø)
src/circom/parser.rs 70.00% <66.66%> (ø)
src/circom/type_analysis.rs 66.66% <66.66%> (ø)
src/process.rs 63.73% <98.57%> (ø)
src/runtime.rs 91.49% <96.48%> (ø)

@brech1 brech1 merged commit 6b68c82 into main Jun 17, 2024
2 checks passed
@brech1 brech1 deleted the test/add-unit-tests branch June 17, 2024 18:12
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.

Compiler doesn't show error to a bad circuit Implement unit tests for basic features
4 participants