[tests] Use the EDN matcher for testing transaction output

[ncalexan]

In #214, I implemented the first parts of #188, which tracks converting Clojure implementation tests into generic EDN format tests that could run against the Clojure implementation and the Rust Mentat implementation.

As I say in #188:

It might be finicky to get the transaction IDs, timestamps, and other ephemeral data correct. I wonder if we can rewrite :db/tx and :db/txInstant in controlled locations to get what we want?

And that's what I did in #214: I translated transaction IDs and certain timestamps into edn::Value::Symbol instances like ?txN and ?msN, and then compared the resulting EDN with test EDN using the same symbols. That is, after reading from the store I post-processed the transaction assertions and datom assertions, etc, looking for particular patterns and combinations to rewrite.

This ticket tracks doing something more general. I'd like to be able to take arbitrary EDN and pattern match it against EDN, without having to postprocess. I want to handle a few things:

• _ matches arbitrary sub-EDN;
• ?name matches sub-EDN, which must be identical each place ?name appears;
• ... matches arbitrary sibling-EDN

For example:

• [_] matches an arbitrary one-element vector
• [_ _]matches an arbitrary two-element vector
• [?x ?x] matches [1 1] and [#{} #{}] but not [1 2] or [[] #{}]
• [1 ...] matches [1] and [1 1] and [1 2 3], but not [2] or [2 1] or #{1 2}.

Note that ... is a valid EDN symbol. Sibling EDN is a little tricky because it needs to handle un-ordered sets and maps, but I think it's do-able.

This is a particularly useful case of a structural EDN differ, as discussed in #220.

[ncalexan]

This is a fun ticket if you know some Rust, but it's probably not a great first bug. It's a good second bug, though!

You'll want to update:

• the test harness around

  mentat/db/src/db.rs

  Line 891 in 4258053

  fn test_add() {

  to use your new differ;
• the debug utilities that the harness uses to not post-process; see

  mentat/db/src/debug.rs

  Line 130 in 4258053

  if a == entids::DB_TX_INSTANT {

  ;
• the text data itself around https://github.com/mozilla/mentat/tree/42580539b8a58f9d10e5891a22112f8c1b697bff/tx/fixtures;
• the Wiki page documenting the test format at https://github.com/mozilla/mentat/wiki/Transacting:-EDN-test-format

[ncalexan]

And... I just hit my first real-world use case where I want this. Concretely, I'm testing a "complex upsert" of the form

[["t1" :db.schema/attribute "t2"]]

The resulting transaction includes an assertion like

[65536 :db.schema/attribute 65537]

with newly allocated entids. It would be great to be able to match those (with ?eN, say) and verify we're getting new entids, etc. (In this case, it's hard to say anything about the ?eN, but it would good to check the form at least.)

[victorporof]

@ncalexan There's a relatively important issue here where the patterns agreed upon cannot themselves be matched as if they were actual symbols in the matched edn. For example, agreeing that _ matches arbitrary subtrees would mean that having an explicit match for a _ symbol would be impossible. Similar issue for ... and ?foo.

If this is something we care about, there seems to be a need for explicitly distinguishing edn values from patterns. Extending the edn spec with #p pattern rules, and perhaps some clear Pattern type? This might be overcomplicating things, but we're otherwise clearly limiting the matcher's capabilities.

[ncalexan]

@ncalexan There's a relatively important issue here where the patterns agreed upon cannot themselves be matched as if they were actual symbols in the matched edn. For example, agreeing that _ matches arbitrary subtrees would mean that having an explicit match for a _ symbol would be impossible. Similar issue for ... and ?foo.

Yep, understood. This is conceptually like "quoting", but in the EDN. I'm happy to just leave this for now, file follow-up, and work out something else later if it's necessary.

[ncalexan]

If you really want to leave this open for generalization, add a trait like:

trait PatternMatching {
    /// Return true if the given pattern matches an arbitrary value.
    fn matches_any_value(pattern: edn::Value) -> bool;
    /// Return true if the given pattern should match arbitrary sibling values.
    fn matches_any_siblings(pattern: edn::Value) -> bool;
    /// Return a symbol and value if the given pattern matches a placeholder.
    fn matches_placeholder(pattern: edn::Value) -> Option<(edn::Symbol, edn::Value)>;
}

then you can implement DefaultPatternMatching on some placeholder type, which just compares to _, ..., and ?name as appropriate.

[victorporof]

@ncalexan On what kind of "placeholder" type would this trait be implemented? I'm having a hard time understanding what exactly you mean by this. A placeholder for some edn::Pattern? edn::Value? I don't see how traits fit into the equation.

[victorporof]

@ncalexan The matches_any_siblings function seems to be tricky to implement due to the fact that edn values have no concept of ancestors nor siblings (even though edn values are basically tree structures). The signature should either accept some "parent" value, or be something like matches_some_children or similar, unless I'm misunderstanding everything.

[ncalexan]

@ncalexan On what kind of "placeholder" type would this trait be implemented? I'm having a hard time understanding what exactly you mean by this. A placeholder for some edn::Pattern? edn::Value? I don't see how traits fit into the equation.

I see now all the trait methods should take references; with that change:

struct DefaultPatternMatching;
impl PatternMatching for DefaultPatternMatching {
    fn matches_any_value(pattern: &edn::Value) -> bool {
        pattern == &edn::Value::Symbol("_".into()) // May need some changes to actually make a `Symbol`.
    }
    ...
}

and then something like:

impl edn::Value {
    pub fn matches(pattern: edn::Value, using: P) where P: Into<Option<PatternMatching>>> -> MatchDetails {
        let using = using.into().unwrap_or(DefaultPatternMatching);
        ...
    }
}