-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge stuff from Hedgehog.Experimental? #182
Comments
❤️ 👍
What would be the reason of having Hedgehog.Experimental then? |
"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.) |
Absolutely. 👍 There's #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). |
Sounds good, I'm in no immediate hurry. 👍 |
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? |
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. |
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
|
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 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 Heck, if everything except |
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. 👍 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 |
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.)
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.
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.
Sure! 👍 |
Also because once we add something, it's there forever.— |
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 ❤ |
I'm in no hurry. Thanks! :) |
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 |
Thanks @nth-commit, that not a bad suggestion :) Also Hedgehog.Extensions is a good name. |
It could be nice to have the auto-generator in its own library, though I'd like to solve hedgehogqa/fsharp-hedgehog-experimental#19 first. Unfortunately I have too many OSS projects and too little time to work on them. Ideas and PRs are welcome! |
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. |
@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 |
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. |
Understood.
In that case, maybe @moodmosaic or @TysonMN have stronger opinions? |
I am in favor of moving everything to the Hedgehog project, but I don't care much either way. |
Perhaps you want to move 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. |
Also, IIRC, the coding style is different between the two codebases(?) Perhaps you'd want to look into that as well. |
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.
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.
👍 |
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 |
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 |
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. |
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. |
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.) |
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. |
Then, perhaps something like this comment is all we want fsharp-hedgehog/src/Hedgehog/Numeric.fs Lines 3 to 4 in 8036e28
|
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. |
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. |
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.
The text was updated successfully, but these errors were encountered: