search

conjure-cp / conjure-oxide

Mozilla Public License 2.0
8 stars 16 forks source link

Suite of integration tests for Minion #249

Open gskorokhod opened 8 months ago

gskorokhod commented 8 months ago

We need a large suite of problems to solve with minion and compare our solutions to conjure (see #247).

Having a large test suite would probably flag up some issues (unsupported Essence features, unsupported Minion constraints, issues with the rewriting engine, issues in minion_rs) and hopefully provide more direction for further work

ChrisJefferson commented 8 months ago

This is probably a long term project, but I strongly believe fuzzing is the only way to get top quality coverage. Once we had a program which could generate valid minion inputs, and tested with those, we found dozens of bugs in minion I'd never have found otherwise.

ozgurakgun commented 8 months ago

I agree that fuzzing will be good and I agree that we are not there yet :)

We probably want a native parser first, before we get to fuzzing.

niklasdewally commented 8 months ago

I agree that fuzzing will be good and I agree that we are not there yet :)

We probably want a native parser first, before we get to fuzzing.

We can fuzz into rust types fairly easily using the abitrary crate (ie from bytes to structs without a parser)

niklasdewally commented 8 months ago

See https://github.com/conjure-cp/conjure-oxide/issues/231 - could do something similar for the conjure oxide ast?

niklasdewally commented 8 months ago

My plan for minion_rs was to use the standard integration tests for "valid models" and "right solutions" and then test safety and ffi and the c++ by fuzzing over the minion_rs ast.

If we fuzz the bindings well, we may not need to fuzz in conjure-oxide itself for Minion stuff, as minion_rs would be more or less safe

gskorokhod commented 8 months ago

I agree that fuzzing will be good and I agree that we are not there yet :)

We probably want a native parser first, before we get to fuzzing.

I think long term it would be useful to have both a Rust native parser for Essence and pretty printing of our AST back to essence. Then it would probably be relatively easy to procedurally generate some valid models and pretty print them to an Essence file that we can feed to old conjure to compare results?

gskorokhod commented 8 months ago

This is probably a long term project, but I strongly believe fuzzing is the only way to get top quality coverage. Once we had a program which could generate valid minion inputs, and tested with those, we found dozens of bugs in minion I'd never have found otherwise.

We should definitely work on fuzzing, but maybe it should be a separate issue?

I was thinking we could start with some existing Essence files just to quickly find some things that aren't working. (At this stage there will probably be obvious bugs in the rules, or some constraints just not being supported etc)

And later, when we have a working base, we can try to test more exhaustively with fuzzing

github-actions[bot] commented 5 days ago

This issue has been automatically closed because it has been inactive for 30 days. Please reopen if you think this issue is still relevant.

ozgurakgun commented 5 days ago

this is ongoing work that probably concerns a few people. at least @niklasdewally and @EEJDempster

niklasdewally commented 5 days ago

I am not sure what exactly the scope of this issue is.

I am adding essence prime tests and tests targeting specific Minion constraints as part of my standard process for adding rules and doesn't need to be done separately.

The only other specific todo I can think of for Minion correctness is fuzzing minion_rs / Conjure Oxide and occasionally checking coverage to see what test cases I've missed.

The former should already be in separate issues..

On 11/11/2024 12:00, Özgür Akgün wrote:

this is ongoing work that probably concerns a few people. at least @niklasdewally https://github.com/niklasdewally and @EEJDempster https://github.com/EEJDempster

— Reply to this email directly, view it on GitHub https://github.com/ conjure-cp/conjure-oxide/issues/249#issuecomment-2468006345, or unsubscribe https://github.com/notifications/unsubscribe-auth/ AKQHNEZEXT7VBLXUMF6AMND2ACL7BAVCNFSM6AAAAABRQXGMRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRYGAYDMMZUGU. You are receiving this because you were mentioned.Message ID: <conjure- @.***>

niklasdewally commented 4 days ago

Related: #171 tracks low level eprime integration tests that target specific minion constraints.