hedgehogqa / fsharp-hedgehog

Release with confidence, state-of-the-art property testing for .NET.
https://hedgehogqa.github.io/fsharp-hedgehog/
Other
272 stars 30 forks source link

Merge stuff from Hedgehog.Experimental? #182

Open cmeeren opened 5 years ago

cmeeren commented 5 years ago

Hedgehog.Experimental has existed in relatively stable form for quite a while now. Could you consider merging some functionality into the core Hedgehog library? For example, the auto-generation of arbitrary types is really nice to have.

Note that when I say "stable" I just mean "hasn't changed in a while". I have no idea how many people actually use Hedgehog.Experimental.

moodmosaic commented 5 years ago

Hedgehog.Experimental has existed in relatively stable form for quite a while now.

:heart: :+1:

Could you consider merging some functionality into the core Hedgehog library?

What would be the reason of having Hedgehog.Experimental then?

cmeeren commented 5 years ago

"Experimental" doesn't sound safe to use. Having some core, important features such as auto-generation part of the main Hedgehog package could make it more attractive to users.

Also, as I understood your comment here, this was the intent from the beginning: For Hedgehog.Experimental to be a proving ground for new features with the intent to promote them to the main Hedgehog library after a while.

At the very least, I think Hedgehog.Experimental should be clearly linked to from this repo's readme and/or tutorial. There is absolutely no mention of Hedgehog.Experimental or "auto" (e.g. auto-generation) anywhere in this repo, so potential Hedgehog users have no clear way of knowing that Hedgehog actually supports e.g. auto-generation - they'd have to search the issues or stumble across Hedgehog.Experimental themselves through other channels.

(Also note that I'm not talking about merging everything, but there are some features that are very convenient such as auto-generation etc. that I think would benefit the main library if included.)

moodmosaic commented 5 years ago

I think Hedgehog.Experimental should be clearly linked to from this repo's readme and/or tutorial

Absolutely. :+1:

There's https://github.com/hedgehogqa/fsharp-hedgehog/issues/138 for this. Feel free to make a pull request, otherwise I may do so (currently I'm travelling so it might take a while before I get to it).

cmeeren commented 5 years ago

Sounds good, I'm in no immediate hurry. šŸ‘

moodmosaic commented 5 years ago

Having some core, important features such as auto-generation part of the main Hedgehog package could make it more attractive to users.

I'm worried that if we add auto-generation inside the core library, people coming from QC might start using just that and start to forget about the Range type and combinatorsā€”an important quality of Hedgehog.


Which other generators from Hedgehog.Experimental could get promoted to the core lirbary?

cmeeren commented 5 years ago

And I worry that people will simply stick to FsCheck if we don't implement auto-generation, because the benefits (for many, not all) of auto-generation might outweigh the lack of integrated shrinking in FsCheck. :)

Furthermore, if people (coming from QC or otherwise) use auto-generation, then either it means it actually makes their lives better, or it means they haven't been properly educated in the possible caveats of using auto-generation vs. specifying everything manually. We can ensure the readme/documentation is good enough to make the second scenario less likely.

Haven't seen the video you linked to yet, will do later.

moodmosaic commented 5 years ago

And I worry that people will simply stick to FsCheck if we don't implement auto-generation

This shouldn't be the caseā€”both tools are fineā€”if you want to use QC from Hedgehog there's hedgehog-quickcheck and if you want to use FsCheck from Hedgehog there's #47 where we can do similar.

The 'vision' for the F# version of Hedgehog is more about staying inline with the Haskell version where possibleā€”supporting state machine testing (#110) for example.

That being said, I think it makes more sense at the moment to

cmeeren commented 5 years ago

Thanks for your patience with my semi-rant. :) I'm coming at this from a completely different angle than you. I've never used Haskell, was introduced to property-based testing through FsCheck and only switched to Hedgehog because having to define Arbitraries for all my gens to get proper shrinking was a big letdown. To be clear, I have no personal vendetta against FsCheck, I'd just like Hedgehog to be better known than it currently is since I find its API and functionality better, and thus would like it to have a solid user base so as to not fade away into oblivion.

Since my angle is so different and my experience with Haskell and pure FP so limited, there's probably a lot of the Hedgehog-specific stuff I don't fully understand, and in my mind it still makes sense to include auto in the main library, but I can appreciate the fact that I only see a limited part of what makes Hedgehog what it is and why its focus is what it is, so I will respect your decision.

Regarding other generators to include: I looked through the Hedgehog.Experimental readme, and I find that I'd like practically everything merged.

If pressed, I can live without the following since they are just trivial convenience gens (though I do like them and would likely implement them in all my test projects anyway if not included, since they make most of my tests notably easier to read):

I find all the rest very useful across my various test projects. What do you think?

(Note that ideally, I'd like the case-insensitive string combinators suffixed with Ci instead of prefixed with i, but both conventions work. They are documented clearly anyway, and I think users won't be confused by any of the two naming variants).

Heck, if everything except auto is merged, I could release a separate stable library with just auto. Perhaps called Hedgehog.Auto. That would be more focused, since auto is perhaps the most significant part of Hedgehog.Experimental (and the most complex implementation-wise). Though my main gripe with Hedgehog.Experimental right now is simply the name; it's not that experimental anymore, and I'd like not to scare users away.

moodmosaic commented 5 years ago

Perhaps, renaming Hedgehog.Experimental to Hedgehog.Auto, AutoHedgehog, etcetera, makes sense. I'm happy to discuss this on that repository (not here); feel free to ping me if you ever open an issue for it. :+1:

Function generation could be on it's own package; we do similar with the Haskell version as well so it's easiest if we're consistent.

We could start by adding shuffle and then slowly and carefully start considering about the next one(s). It doesn't have to be all at once.

cmeeren commented 5 years ago

As I understand you, you want to be careful with adding stuff into the main Hedgehog library because you want to keep it in line with the Haskell version. As I said, even though I don't understand the reasoning behind that (it's two completely different languages/ecosystems/runtimes), I will respect your decision and won't pressure you to merge stuff. šŸ‘

When it comes to splitting Hedgehog.Experimental into separate packages, I don't necessarily think that's a good idea. Sure, focused packages are nice. But Hedgehog.Experimental exists to add convenience to testing with Hedgehog, and I assume that people don't really want to install/update N packages to get N useful generators if they could just as well have installed a single package with all the generators. Furthermore, the maintenance of Hedgehog.Experimental is basically zero, so it's not a problem for me keeping all the convenience combinators in a single package. (Having separate packages would just add overhead.)

Perhaps, renaming Hedgehog.Experimental to Hedgehog.Auto, AutoHedgehog, etcetera, makes sense.

I have no problems with renaming Hedgehog.Experimental, but unless everything other than auto-generation is extracted/merged, it would need to be a general name that encompasses all the generators. As @jystic mentioned in this comment, the intention behind the name Hedgehog.Experimental was to merge combinators into the main library. I'd rather call it something like Hedgehog.Extra or Hedgehog.Batteries to indicate that it's safe/stable. It's still possible to merge some generators into the main library and obsolete them from Hedgehog.Whatever, but the main purpose of the library won't be to be a proving ground, but instead provide convenience generators to make testing easier.

Function generation could be on it's own package; we do similar with the Haskell version as well so it's easiest if we're consistent.

I don't really think function generation is worth its own package. It would just be a handful of lines of actual implementation code, and a whole lot of package/infrastructure boilerplate, plus one more library users would have to install, as mentioned above.

We could start by adding shuffle and then slowly and carefully start considering about the next one(s). It doesn't have to be all at once.

Sure! šŸ‘

moodmosaic commented 5 years ago

As I understand you, you want to be careful with adding stuff into the main Hedgehog library because you want to keep it in line with the Haskell version

Also because once we add something, it's there forever.ā€”shuffle is surely a good start though.

moodmosaic commented 5 years ago

I'll start by adding Gen.shuffle, and I'll update also the README as discussed above.

I apologize for moving slow šŸŒ feel free to send PRs in case you want to get this done faster ā¤

cmeeren commented 5 years ago

I'm in no hurry. Thanks! :)

nth-commit commented 5 years ago

Hey @cmeeren thanks for your work with Hedgehog.Experimental, it's a package that I have found myself reaching for often.

Just to weigh in with another opinion, I think it's nice keeping the core library "light", much fewer concepts to understand when you first pick it up. By light I mean the basic functionality of generators/properties, and a few generators of primitive types. The functions you have written that over Range e.g. eList, while useful, I think would be better suited in something like Hedgehog.Extensions. Perhaps the auto stuff could go in another library again?

cmeeren commented 5 years ago

Thanks @nth-commit, that not a bad suggestion :) Also Hedgehog.Extensions is a good name.

cmeeren commented 5 years ago

It could be nice to have the auto-generator in its own library, though I'd like to solve https://github.com/cmeeren/fsharp-hedgehog-experimental/issues/19 first. Unfortunately I have too many OSS projects and too little time to work on them. Ideas and PRs are welcome!

moodmosaic commented 3 years ago

Once we get v0.9.0 out, and before we reach v1.0.0, we should look into this. There's definitely many stuff in the experimental project that are battle hardened at this point.

ghost commented 3 years ago

@cmeeren Do you have a few things in mind that you'd like to move from experimental over to Hedgehog proper? I'd like to do this for 0.12.0.

cmeeren commented 3 years ago

To be honest I've practically abandoned active development of Hedgehog.Experimental and now just fix bugs as they are reported (or at least welcome PRs with fixes). I actively use it though. I don't really care that much whether the stuff there stays there or gets merged into the main package. Personally I'm fine with the way things are now.

ghost commented 3 years ago

To be honest I've practically abandoned active development of Hedgehog.Experimental …

Understood.

I don't really care that much whether the stuff there stays there or gets merged into the main package. Personally I'm fine with the way things are now.

In that case, maybe @moodmosaic or @TysonMN have stronger opinions?

TysonMN commented 3 years ago

I am in favor of moving everything to the Hedgehog project, but I don't care much either way.

moodmosaic commented 3 years ago

Perhaps you want to move auto if you're heavily using it.

But I wouldn't move each and every combinator as that'll increase maintenance and it'll make it hard(er) to draw a line; people might come in wanting to add more and more combinators.

moodmosaic commented 3 years ago

Also, IIRC, the coding style is different between the two codebases(?) Perhaps you'd want to look into that as well.

ghost commented 3 years ago

Perhaps you want to move auto if you're heavily using it.

But I wouldn't move each and every combinator as that'll increase maintenance and it'll make it hard(er) to draw a line; people might come in wanting to add more and more combinators.

On the combinator note, I've seen exactly this with the reactive extensions. There are probably at least 100 different combinators there for very specific use cases. We definitely want to avoid this at all costs.

Also, IIRC, the coding style is different between the two codebases(?) Perhaps you'd want to look into that as well.

I was thinking maybe we bring the entire repo in as another project in the solution at first as having two completely separate repos doesn't make a ton of sense to me here. I'd be fine keeping the packaging the exact same as it is now, etc, but maybe we can have one less repo in the hedgehogqa org.

I am in favor of moving everything to the Hedgehog project, but I don't care much either way.

:+1:

ghost commented 3 years ago

What would be the proper way to handle this "mechanically"? Would we just copy/paste the contents of the repo? Or do we want to preserve the history of fsharp-hedgehog-experimental by merging an "unrelated history" into a sub-folder in this repo?

moodmosaic commented 3 years ago

A git submodule would be preferred, no?

ghost commented 3 years ago

A git submodule would be preferred, no?

A submodule would work, for sure. The issue there is that it remains a separate repo.. I agree with keeping Experimental as a separate project, but I think it can happily coexist in the fsharp-hedgehog repo. The separate repo just adds overhead all around šŸ¤·šŸ¼

cmeeren commented 3 years ago

Concerning merging repos: Feel free. šŸ‘ But if this is done, I think I'll consider what is now in Experimental to be maintained by the current maintainers of this (the main) repo, and I'll withdraw from maintenance of Experimental. (Which, again, is not particularly actively maintained now anyway.) I'll transfer the NuGet package ownership to you in that case.

In any case, I have no strong desire to "own" (commit-wise) whatever is now in Hedgehog.Experimental. If archiving that repo and copy-pasting the code into this repo is the easiest solution, go for it. Git submodule seems a suboptimal solution, maintenance-wise.

TysonMN commented 3 years ago

I would prefer not to have a submodule, but I won't advocate against it.

I don't think the history is important, so I think copy-pasting the code would be fine to me.

moodmosaic commented 3 years ago

I don't think the history is important, so I think copy-pasting the code would be fine to me.

I think history is important because you'll be moving code that's written by someone else (@cmeeren) and @cmeeren might not be around (at some point/after a while) to answer questions and then git history is all we got.

If we just move the code over, we can at least mark the git commit SHA-1 and then make readonly/archive the experimental repo so that the history there can't be altered. (So in this case whether is a submodule or not won't matter.)

TysonMN commented 3 years ago

Sure, we can keep that repository around. What I meant is that I don't think we need you are the history of that code in this repository.

moodmosaic commented 3 years ago

Then, perhaps something like this comment is all we want https://github.com/hedgehogqa/fsharp-hedgehog/blob/8036e28f7fff61efdda2119cd9a7cbd1963d13c3/src/Hedgehog/Numeric.fs#L3-L4

TysonMN commented 3 years ago

Yes, that comment could be at the top of each copied file or in a single file at the root of the folder containing all copied files.

cmeeren commented 3 years ago

Concerning different code styles: The F# code formatting guidelines just received an overhaul/facelift. They now strongly recommend the use of Fantomas with default settings for formatting code. (In VS, the "F# Formatting" extension enables this.) You could consider simply formatting everything with Fantomas, for consistency.