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

Refactor/testing #880

Merged
merged 4 commits into from
Jul 5, 2023
Merged

Refactor/testing #880

merged 4 commits into from
Jul 5, 2023

Conversation

Patrik-Stas
Copy link
Contributor

@Patrik-Stas Patrik-Stas commented Jun 9, 2023

  • Aries-vcx: Streamlines and deduplicates test initialization
  • Libvcx: Minimize usage of "handle" vocabulary in favor of working with trait object components. This was preliminary work to enable running libvcx with different than vdrtools profile

Aries-vcx Alice/Faber testing

  • Implement create_faber similarly like we had create_alice, which construct the test agent with regard to to feature flags. This significantly increases coverage of issuer/verifier tests running modular libs profile.

  • Use SetupProfile as basis to build out components for Faber/Alice (create_faber/create_alice) in integration tests in favor of logic duplicated in setup_modular_profile, setup_indy_wallet, setup_indy_wallet (now deleted). This again significantly increases coverage for different profile feature flags

Libvcx

  • Rework global storage/Profile handling in libvcx_core. Remove notion of "handles" from libvcx global state, instead is replaced with dealing with individual components (base wallet, anoncreds etc.). This opens up door for swapping different implementation on libvcx level, such as using modural libs components

  • Remove testing feature infra from libvcx, run libvcx unit tests in CI (this was dropped somewhere along the way)

  • Decrease usage of global "settings" variables used in libvcx tests

Various smaller changes

  • removal of unused or under-utilized "Setup" objects
  • removal of commented tests
  • removal of low value tests - typically 1 or 2 tests using specific "Setup" object (which itself would require refactoring) while not delivering much value - removed tests along with the setup
  • split Faber and Alice integration test agents to separate files
  • Rename SetupPool to SetupPoolDirectory and change behaviour, such that it doesn't connect to pool, only prepares path to store temporary pool genesis file. Previously in some tests, we been doing extra unused connection to pool.
  • Delete TestSetupCreateWallet and the single libvcx (deprecated component) test utilizing it 'test_vcx_init_called_twice_passes_after_shutdown' - wallet/pool re-initialization is still indirectly covered by NodeJS integration tests
  • Delete "SetupPoolConfig" and libvcx test utilizing it "test_full_init" (similar story like above)
  • Use unique poolname (for vdrtools bases profiles) to increase resilience of test suites
  • Change state_vcx_shutdown behaviour - ignore delete argument, which deletes wallet and pool. This was unused and is practically rather dangerous than useful.

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2023

Codecov Report

Merging #880 (b0f2e49) into main (57c57e0) will decrease coverage by 2.64%.
The diff coverage is 75.78%.

@@            Coverage Diff             @@
##             main     #880      +/-   ##
==========================================
- Coverage   49.54%   46.91%   -2.64%     
==========================================
  Files         436      438       +2     
  Lines       35163    34826     -337     
  Branches     7659     7578      -81     
==========================================
- Hits        17423    16340    -1083     
- Misses      12450    13690    +1240     
+ Partials     5290     4796     -494     
Flag Coverage Δ
unittests-aries-vcx 46.91% <75.78%> (-2.60%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aries_vcx/src/common/credentials/mod.rs 75.75% <ø> (ø)
aries_vcx/src/common/keys.rs 61.25% <ø> (ø)
aries_vcx/src/common/ledger/transactions.rs 42.99% <ø> (ø)
...vcx/src/common/primitives/credential_definition.rs 59.91% <ø> (ø)
...src/common/primitives/revocation_registry_delta.rs 54.71% <ø> (ø)
aries_vcx/src/common/proofs/proof_request.rs 60.71% <ø> (ø)
aries_vcx/src/common/proofs/prover/prover.rs 45.71% <ø> (ø)
...es_vcx/src/common/proofs/prover/prover_internal.rs 79.79% <ø> (-1.53%) ⬇️
...cx/src/common/proofs/verifier/verifier_internal.rs 67.71% <ø> (ø)
aries_vcx/src/common/test_utils.rs 93.57% <ø> (ø)
... and 49 more

... and 32 files with indirect coverage changes

@bobozaur bobozaur force-pushed the change/remove-profiles branch from 02ff8d5 to 5d213d5 Compare June 13, 2023 14:57
Base automatically changed from change/remove-profiles to main June 14, 2023 04:18
@Patrik-Stas Patrik-Stas force-pushed the refactor/testing branch 2 times, most recently from 8a4fa46 to 0190cf1 Compare June 16, 2023 10:44
@Patrik-Stas Patrik-Stas force-pushed the refactor/testing branch 6 times, most recently from 371d3ac to dbf4014 Compare June 27, 2023 17:56
@Patrik-Stas Patrik-Stas force-pushed the refactor/testing branch 3 times, most recently from 18d1522 to 402bdd9 Compare June 28, 2023 07:59
bobozaur
bobozaur previously approved these changes Jul 4, 2023
Copy link
Contributor

@bobozaur bobozaur left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@bobozaur bobozaur merged commit cc6ac0c into main Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants