Simple constant folding optimization as an example

[rtjoa]

Looks good! I like that we're moving away from egglog programs as strings.

egglog_test seems to do two separate things. What if we split it into the following functions? (Separation of concerns; we can still encourage people to test with both styles)

egglog_test_opt(
  build: &str,
  check: &str
)

egglog_test_interp(
  pgrm: TreeProgram,
  input: Value,
  expected: Value
)

Another common test pattern, which desugars to egglog_test_opt, could be

egglog_test_optimized_to(
  pgrm: TreeProgram,
  epxected: TreeProgram
)

[oflatt]

I considered separating these two, but we should almost always be calling both. This way, you won't forget to.

[rtjoa]

I considered separating these two, but we should almost always be calling both. This way, you won't forget to.

I think tests will come out cleaner and be more readable if they are separate. If it's good style to test both, this should happen during review.

Also note that it's odd we expect all test programs to have the same expected value - turning the param from a vector of programs to a single program (or: vector of program-output pairs) will encourage more diverse testing.

[rtjoa]

Here's a "test both" alternative that doesn't do two separate things:

egglog_unit_test_opt(
  pgrm: TreeProgram,
  expected: TreeProgram
  input_output_tests: Vec<Pair<Value, Value>>
)

It can first interpret and check the i/o pairs on both pgrm and expected. If there's a discrepancy, there's an issue in the test or the interpreter!

Then, if pgrm is not rewritten to expected, we catch a bug in the optimizations.

Note that it can be implemented neatly using the functions I suggested in my first comment.

[oflatt]

We should add that too, but a lot of our tests involve strait up egglog code (f.e. analysis tests) Having a list of values would also be nice but for now this is good enough

[ajpal]

I considered separating these two, but we should almost always be calling both. This way, you won't forget to.

I think tests will come out cleaner and be more readable if they are separate. If it's good style to test both, this should happen during review.

Also note that it's odd we expect all test programs to have the same expected value - turning the param from a vector of programs to a single program (or: vector of program-output pairs) will encourage more diverse testing.

+1

[oflatt]

Chatted with @ajpal about this For now lets go with the existing one, we can add @rtjoa's egglog_unit_test_opt as needed