evomimic / map-holons

3 stars 1 forks source link

Add get_holon_by_id dance and match_db_content Dance Test Step #76

Closed evomimic closed 4 months ago

evomimic commented 4 months ago

This enhancement delivers the get_holon_by_id dance and the match_db_content test step. The latter allows expected database content to be compared against actual database content. It can be used at various stages within a test case. The get_holon_by_id dance is used to implement the match_db_content test step and therefore, is tested by that step (without needing to introduce a separate get_holon_by_id test step).

Current State

In order to compare expected vs. actual db content, we need some way of accumulating the set of expected holons that have been committed within a test case. Issue #69 also enhanced the commit dance function to return the set of Holons created by that commit, but the test_commit function is not currently doing anything with those results.

The get_all_holons dance gets ALL of the holons in the database. To provide the ability to match content for a subset of holons, we need the ability to retrieve individual holons. The get_holon_by_id dance is one we need anyway, so we will bundle the introduction of that dance into this enhancement.

Proposal

Dances Zome

pub enum RequestBody {
    None,
    Holon(Holon),
    HolonId(HolonId), // <= NEW
    ParameterValues(PropertyMap),
    Index(StagedIndex),
}
pub enum ResponseBody {
    None,
    Holon(Holon), // <===== NEW
    Holons(Vec<Holon>), // will be replaced by SmartCollection once supported
    // SmartCollection(SmartCollection),
    Index(StagedIndex),
}

Holons Adapter

/// *DanceRequest:*
/// - dance_name: "get_holon_by_id"
/// - dance_type: Standalone
/// - request_body:
///     - HolonId(HolonId)
///
/// *ResponseBody:*
///     - Holon(Holon)
///

Enhance Dance Testing


This function iterates through the expected_holons vector supplied as a parameter and for each holon:

- builds a `get_holon_by_id` DanceRequest for that holon
- calls the conductor to dance that request
- checks that the holon return matches the expected holon

#### Enhance Tests

- [x] Add `match_db_content` test step to the `simple_create_test_fixture` test case implementation after the commit step, passing in the `expected_holons` vector.
- [x] In the test_data_types.rs file, 
    - [x] `MatchSavedContent` as a variant in the `DanceTestStep` enum definition
    - [x]  add its fmt::Display implementation
    - [x] implement an `add_match_db_content_test_step()` function
            `add_match_db_content_test_step(&mut self) -> Result<(), 
              HolonError>` 
- [x] In dance_tests.rs file,
    - [ ] Add match clause for the `MatchSavedConent` step, which introduces an execute_match_db_content function 
            that iterates over test_case.created_holons to check that each "expected_holon" matches the fetched_holon from the response body of the dance call.
### Definition of Done:
- [ ] After adding `match_db_content` test step to the `simple_undescribed_create_holon_test`, test case **passes**
evomimic commented 4 months ago

The goal of this test step is to ensure the actual holons retrieved from the persistent store in this step match the expected holons accumulated during prior steps in this test case. Some challenges have arisen during the design and implementation of this enhancement.

Challenges

Two questions arise:

  1. How do we accumulate expected holons and make them available to the match_db_content test step?
  2. What does match mean?

There are two sets of holons that are candidates for the expected holons set:

  1. fixture holons -- the set of holons created within the test fixture that are used as the source of the create/update test steps. This set could be accumulated within the test fixture, but this is not currently being done.
  2. created holons -- the set of holons created as a result of commit steps within the test case execution. In the current implementation of this issue, the execute_commit function was enhanced to accumulated saved holons in test_state.created_holons.

However, the holons in neither set will exactly match the holons in the actual_holons set.

Proposal

  1. Adjust our definition of match so that we are only matching on the essential content of the holons.
  2. Use the holons accumulated during test execution in test_state.created_holons as the expected holons.

For the first part of the proposal...

pub struct EssentialHolonContent {
    pub property_map: PropertyMap,
    pub relationship_map: RelationshipMap,
    key: Option<MapString>,
    pub errors: Vec<HolonError>,
}

Note that EssentialHolonContent excludes saved_node and the Holon's state variables.

Use this struct when matching expected and actual holons:
assert_eq!(expected_holon.essential_content(), actual_holon.essential_content())

The second part of the proposal is a bit trickier, because it needs to take into account:

The exact design for handling these cases is TBD.

dauphin3 commented 4 months ago

Proposal

We could have the DancesTestCase contain the fixture holons

Comments

dauphin3 commented 4 months ago

The second part of the proposal is a bit trickier, because it needs to take into account:

multiple commits, partial commits (where some staged_holons are successfully saved and others are not), recommits where staged holons that had errors in one commit are corrected and then successfully saved as a result of a subsequent commit.

Perhaps this should be a separate issue? @evomimic

evomimic commented 4 months ago

Go ahead with fixture based approach.

dauphin3 commented 4 months ago

Actually may not need to add to the fixture.. EssentialHolonContent fixed the issue since execute_match_db_content takes a mutable reference to test_case and can get 'expected_holons' from the test_case.created_holons. A cautionary note is that this is possible because this is the last step in the order (it's worth noting that maybe this could be pulled out into the test file itself - it all depends on whether other steps would come after it and need to access the 'created_holons" field, in which case we we need a top level Vec to push to after each commit step)

@evomimic pushed changes, test as is passes.

evomimic commented 4 months ago

As currently, coded the complications noted above have not been addressed:

But the fix for these probably won't involve the execute_match_db_content logic per se. The changes will need to be the other test step execution functions. So I'm OK with this as coded for now.

The match_db_count step is failing while attempting the get_holon_by_id dance. The source of the error seems to be on guest-side code. To aid debugging this, I approved PR #82 and intend to approve PR #72, so we can use guest-side tracing to help debug the issue.