StateM.DSL refactor

[x4lldux]

Pull request resolving #142

[alfert]

@x4lldux, if I see this right, then you changed the existing DSL implementation. Since we wanna break the API, this is not friendly to our users. I would suggest that if we break the API, then we should introduce this via a new module (that's why I mentioned DSL2 earlier in our discussions, see #142 (comment)) and simply mark the old DSL with a hard- or soft-deprecation mark (to be discussed and decided).

This gives use the choice of using the newer, nicer DSL with better reporting or stay with the old DSL. If we don't give them that choice, they are required to change their tests or stay with the old version of PropCheck or even leave it behind and move to something different. Let's play nicely with our users!

I will take a closer look on your implementation later!

[alfert]

language: The sequence

[x4lldux]

"sequence" is both countable and uncountable so I'm not sure it's wrong, but OK, I'll change it.

[alfert]

@x4lldux Good work, thanks! I marked some language improvements in the documentation and please re-introduce the type documentation.

And a general comment on discussing the command generation: From my perspective the mastership of defining good models is in generating and properly handling senseless examples. This is contrary to your point of not generating the delete_user command if no users are allowed. It is often interesting to see how the system reacts if a non-existing user is to be deleted. Should result in a proper error message, but does it happen?

[x4lldux]

@x4lldux, if I see this right, then you changed the existing DSL implementation. Since we wanna break the API, this is not friendly to our users. I would suggest that if we break the API, then we should introduce this via a new module (that's why I mentioned DSL2 earlier in our discussions, see #142 (comment)) and simply mark the old DSL with a hard- or soft-deprecation mark (to be discussed and decided).

Yes, that's a breaking change, that's why I've mentioned we would need to go to v2.0 (assuming semantic versioning). And one breaking change was already introduced in run_commands, though it will touch only some users.
Either way, we have two scenarios here. a) If we rename this PR as a DSL2 (and by the way, versioned module names are ugly 😜), deprecate DSL, and after few releases remove it, than it will be a breaking change needing we go to v2.0. At which point we'll end up with just a DSL2 module, but no reason for the "2" postfix, so it will be reasonable to rename it back to DSL - another breaking change (v3.0 ?!).
b) Or we replace it with the new implementation, call it DSL and go v2.0. Both scenarios lead to v2.0 (again, assuming semver).

We can, though, go with b) but rename current implementation to to say DSL.Old and deprecate it, so current user will have a choice to stick to the old implementation with a minimal change (just renaming to use PropCheck.StateM.DSL.Old) or refactoring the tests for the new implementation. And since DSL.Old will be already deprecated in v2.0, we could later remove it without a breaking change, I think..

[x4lldux]

And a general comment on discussing the command generation: From my perspective the mastership of defining good models is in generating and properly handling senseless examples. This is contrary to your point of not generating the delete_user command if no users are allowed. It is often interesting to see how the system reacts if a non-existing user is to be deleted. Should result in a proper error message, but does it happen?

I'm not advocating it shouldn't be done, I said "there might not be much sense". Straying from the happy path is a good practice! And I also do it. But deleting a user when no users are present, or closing a file while it's not open yet are low hanging fruits - easy to spot by a developer. And generating a commands with easily spotable nonsensical sequences, where all commands are equal, will reduce the chances of finding hidden bugs. So focusing on sequences that on the first look (and sometimes second and third) should work is often more valuable.

In that regard both implementation (command_gen/1 and weight/1+args) are equivalent! Both allow for both methodologies. Though, in command_gen/1 it's easier to follow because there's less context switching and source-file jumping around - you have all possible sequences (with their args) in one place.

[alfert]

... should work is often more valuable.

Fair enough. It was not a critique of the DSL, it was merely remark on the justification, meant jokingly, but this obviously was not the well perceived. sorry!

[alfert]

Naming things is one of the hardest problems in computer science :-)

We can, though, go with b) but rename current implementation to to say DSL.Old and deprecate it, so current user will have a choice to stick to the old implementation with a minimal change (just renaming to use PropCheck.StateM.DSL.Old) or refactoring the tests for the new implementation. And since DSL.Old will be already deprecated in v2.0, we could later remove it without a breaking change, I think..

IMHO, that's not a good path, since all users of the next version will then have to change their DSL-based tests since it is completely incompatible due to command_gen vs. args. You are correct that numbered module names are ugly. So, I would propose to call your approach ModernDSL, BetterDSL or NewDSL (I prefer modern) to differentiate between the original and the refactored DSL. We should in any case add a deprecation tag on the existing DSL module to give a clear indication that the old DSL is not the proper way of doing things and that it will vanish in the future (in v2.0.0).

What do you think? A different name?

[x4lldux]

Fair enough. It was not a critique of the DSL, it was merely remark on the justification, meant jokingly, but this obviously was not the well perceived. sorry!

No, I didn't took it as a critique or anything. I just tried to explained what I meant behind it. Mainly for "prosperity" in case some users will read it. And because at the beginning I was spending a lot of time testing those low hanging fruits, mostly testing validation and error handling instead of focusing on the core... and then bugs happened. So no apology needed, no offense taken. It was merely a warning on to often taking that route, based on own experience.

[x4lldux]

Naming things is one of the hardest problems in computer science :-)

Yes, it's one of the two most hardest things. Alongside cache invalidation and off-by one errors 😜

So, I would propose to call your approach ModernDSL, BetterDSL or NewDSL (I prefer modern) to differentiate between the original and the refactored DSL

That's not really any different. When we're eventually on v2.0 and DSL is dropped and we went with ModernDSL, BetterDSL or NewDSL, then one might ask why is it "new" - where's the old? Or, why is it better? Better than what? Also, in that situation it might be prudent to rename (Modern|Better|New)DSL to just DSL, which will also require changes in user's tests.

Also, with b)+DSLOld, there's not much that we will be requiring from users. We'll basically make them to do "replace in project", a feature most code editors have built in, or running a sed -i 's/use PropCheck.StateM.DSL/use PropCHeck.StateM.DSLOld/' *_test.exs command.

[x4lldux]

There's one thing that came to me, though it's hacky a little bit.
We can hide new dsl in side of StateM, but behind an option, like: use PropCheck.Statem, :dsl.
Doing use PropCheck.StateM would run StateM's __using__ as it looks now and everything would work as usual where users are expected to define all the callback of the current StateM.
Doing use PropCheck.StateM, :dsl would run this PR's version of __using__, which registers compilation hooks and users are expected to only define initial_state and command_gen callbacks and use defcommand.

Though, we will have to put an emphasis on creating a clear documentation for those two modes.

[alfert]

I understand your thoughts. Currently I am not on the trip towards a 2.0 version which would imply arbitrary and incompatible changes in the API. From that perspective, I would prefer to be conservative regarding the name. ModernDSL stands for a fresh view on that topic and distinguishes it from my older approach. The using approach with parameter :dsl feels very confusing and has its difficulties in the documentation to make all these alternate APIs clear. That's somewhat similar to Erlang's :queue module where each function appears twice with different names and parameters. That's a bad compromise we should not copy.

When we eventually release a 2.0 version, we can do exactly what you said: We bury the old DSL, rename ModernDSL to DSL and give the advice to use renaming commands as a procedure to migrate old 1.x source to the new 2.0 world.

But until then, I would like to provide both versions to our users, so that they can still use the old DSL if they don't want to change their system but reap the benefits of a newer and enhanced versions of PropCheck.

My proposal for the name is ModernDSL. But I am open for others.

[x4lldux]

OK. Let's do this.

How about StateMDSL, without a dot. Or StateM.EDSL for embedded domain specific language, which technically it is 'cause it's built ontop of Elixir.
Another candidate is StateM.ModelDSL or just StateM.Model.

[alfert]

ModelDSL sounds good, only Model is - as is StateMDSL too confusing syntactically and semantically. EDSL could also be enhanced DSL, but I would go for ModelDSL.

Thanks for your effort!

[x4lldux]

I'm not sure I would go as far and call it "enhanced", maybe just "more of the same" 😜 As it's just a thin wrapper around StateM.

Yeah I also like: StateM.EDSL and StateM.ModelDSL. But since naming is hard, maybe let's ask other's too. @evnu @rodrigues what do you think?

[evnu]

But since naming is hard, maybe let's ask other's too.

I think the name must be easily distinguishable from StateM.DSL to avoid confusion. As a user, I would probably need some time to understand the difference between StateM.DSL and StateM.EDSL, if I did not know about the discussion here. I would be ok with StateM.ModelDSL, although it took me some time to realize that the M in StateM does not mean 'model' (then it would be StateModel.ModelDSL, which looks silly :)). Let me add another contender: StateM.Future.DSL. This would clearly show that this is where the future is and can be renamed to StateM.DSL with 2.0. This suggestion is a similar direction like ModernDSL.

In any case, I think I would actually prefer if PropCheck simply moved to v2.0 and reused the name. StateM.DSL seems to be the most appropriate name for what it is. A new and enhanced DSL justifies a breaking change in my book. But of course this is not up for me to decide :)

[x4lldux]

PropCheck.StateM.Future.DSL... now that's a mouthful! We want them to be using it now ;)

[alfert]

Thanks everybody for your thoughts. Then I would prefer ModelDSL and I will think again about a 2.0 version. I have also some ideas going on concerned with the Pulse functionality of Erlang QuickCheck (i.e. for concurrency issues). That would definitively a good point for a 2.0 version.

[x4lldux]

@alfert would you mind opening an issue about those Pulse ideas? I was under impression that Pulse was supposed to be open-sourced under BSD but never actually did.

[alfert]

opening an issue about those Pulse ideas?

Yes, I will. The current changes are only the initial ones for enabling a scheduler. I never found an open source version of Pulse, but there are the scientific papers and the tutorials. So, we should be able to implement something similar.

[alfert]

@x4lldux, would you mind todo the renaming? I assumed after that we can merge this PR or is something else to be done?

[x4lldux]

@alfert yeah, sure. I'm currently out of town. I'll do it in two days.

[evnu]

I never found an open source version of Pulse

I think https://concuerror.com/ is such an implementation, or at least implements part of what's needed for that. I tried that some years ago, but did not get far with it. Note that the people behind PropEr implemented concuerror.

[x4lldux]

@alfert should the DSL be deprecated or soft-deprecated (if that's even possible on a module level)?

[alfert]

@x4lldux I think soft-deprecation is enough for now. But I can do this for you, i.e. advertising the new DSL implementation.

[alfert]

I think https://concuerror.com/ is such an implementation

Thanks for the reference, I almost forgot about it. I also used concuerror several years ago, I think, it was when the first versions came along and I also did not come that far.

concurerror is a fundamental different beast, since it explores and executes all schedulings whereas the QuickCheck approach samples a few. It is effectively a variant of a model checker, and these tools have always the challenge of memory consumption and processing time they need for exploring all possible runs. But I should give it a try to understand if it is usable now and if the effort for re-implementing Pulse is worthwhile (except for the fun of it).

What I already found after reading the faq is that they also consider ETS operations, time outs and calls to non-deterministic functions such as time() or now() and have special handling for them. That is an interesting thought.

[x4lldux]

Can anyone explain why adding few @deprecated attributes causes dializer to fail in an unrelated function?! 😖
Fixed. I suspect that rewriting tests to use ModelDSL instead of DSL changed something in success typing.

[alfert]

Perfect, thanks @x4lldux ! 💚