Integration tests DAG

[stokhos]

This PR closes #36

[kneasle]

Wow! Amazing work! If the code could be streamlined a bit, then that would be excellent and I'd be happy to merge.

[stokhos]

I'm a little confused with your feedback. (I'm not a native English speaker)
So basically, what I need to improve are:

1. define a run_test_err function
2. some tests includes multiple sub-tests. It's better to move them out.

[kneasle]

That's fair - reading back I see that my feedback is not particularly helpful. What I was trying to say is that the tests are great but the code could be made a lot shorter (and at the same time, easier to read). The following is hopefully clearer (if a bit long because of the code examples):

The code currently has a lot of repeating patterns that I think should be simplified:

1. All of the tests are currently written as making a load of local variables and then passing them into run_test, like:

   let start_tree = <StartTree>;
   let start_cursor_location = <StartCursor>;
   let action = <Action>;
   
   let expected_edit_result = <ExpResult>;
   let expected_tree = <ExpTree>;
   let expected_cursor_location = <ExpCursor>;
   
   run_test(
       start_tree,
       start_cursor_location,
       action,
       expected_edit_result,
       expected_tree,
       expected_cursor_location,
   );

   but we could just pass these directly into run_test and avoid a lot of code (as well as removing the type annotation):

   run_test(
       <StartTree>,
       <StartCursor>,
       <Action>,
       <ExpResult>,
       <ExpTree>,
       <ExpCursor>,
   );

2. CursorPath::from_vec(vec![]) should be written CursorPath::root().

3. Some of the tests are testing cases which cause EditErrs. In these cases, the code follows this pattern (notice that start_tree and start_cursor_location are always the same as expected_tree and expected_cursor_location because a failed action should not make any change to the tree):

   let start_tree = <StartTree>;
   let start_cursor_location = <StartCursor>;
   let action = <Action>;
   
   let expected_edit_result: (bool, Result<EditSuccess, EditErr>) =
       (false, Err(<SomeError>));
   let expected_tree = <StartTree>;
   let expected_cursor_location = <StartCursor>;
   
   run_test(
       start_tree,
       start_cursor_location,
       action,
       expected_edit_result,
       expected_tree,
       expected_cursor_location,
   );

   In this situation, it would be good to funnel all the 'error tests' into their own version of run_test which could look something like:

   /// Run a test that is expected to fail
   fn run_test_err(
       tree: TestJSON,
       cursor_location: CursorPath,
       action: Action,
       exp_error: EditErr,
   ) {
       let arena: Arena<JSON> = Arena::new();
       let root = tree.add_to_arena(&arena);
       let mut editable_tree = DAG::new(&arena, root, cursor_location);
   
       assert_eq!(
           (false, Err(exp_error)),
           editable_tree.execute_action(action),
           "Unexpected result of the action."
       );
       assert_eq!(
           tree,
           editable_tree.root(),
           "Failed action changed the tree."
       );
       assert_eq!(
           editable_tree.current_cursor_path, cursor_location,
           "Failed action changed the cursor location."
       );
   }

   run_test_err doesn't take expected_tree and expected_cursor_path as parameters for the reasons explained above.

   Now, each test case can turn into

   run_test_err(
       <StartTree>,
       <StartCursor>,
       <Action>,
       <SomeError>,
   );

   which is much easier to read and understand.

Thanks for making the tests by the way 😁 - I think they'll make Sapling much more stable.

[stokhos]

You are welcome. I'm also learning a lot by contributing to sapling.

[kneasle]

I'm also learning a lot by contributing to sapling.

I definitely think that Sapling is the ideal size for someone learning Rust/open source - it's big enough to be interesting but it's also small enough that it's quite easy to understand.

It's also really nice not to have to do all the little things myself - I can be pretty sure that any good-first-issues will be snapped up and done! 😁

[kneasle]

Awesome! Merging now.