Skip to content
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

πŸ“ Document how to use context in examples #3458

Merged
merged 3 commits into from Dec 11, 2022

Conversation

tskj
Copy link
Contributor

@tskj tskj commented Dec 8, 2022

This PR addresses the issue raised here. I would prefer a proper fix in the library itself, but this at least gives the users the tools to circumvent the bug themselves!

I have added the suggested piece of code and short explanation to both parts of the docs so that they're easy to find. The idea is that users shouldn't have to stumble over this and have to dig through old issues to potentially find a fix, and can ideally be warned early enough that they circumvent the issue all together.

Please feel free to suggest better, friendlier, or clearer wording, or other places I should add this as well!

Category:

  • ✨ Introduce new features
  • πŸ“ Add or update documentation
  • βœ… Add or update tests
  • πŸ› Fix a bug
  • 🏷️ Add or update types
  • ⚑️ Improve performance
  • Other(s): ...

Potential impacts:

  • Generated values
  • Shrink values
  • Performance
  • Typings
  • Other(s): ...

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 8, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 866a8e9:

Sandbox Source
Vanilla Configuration
@fast-check/examples Configuration

Copy link
Owner

@dubzzz dubzzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, I'd definitely go for this one. I will merge soon

@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #3458 (866a8e9) into main (1385007) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3458      +/-   ##
==========================================
- Coverage   95.51%   95.49%   -0.02%     
==========================================
  Files         213      213              
  Lines        5842     5842              
  Branches     1076     1076              
==========================================
- Hits         5580     5579       -1     
- Misses        262      263       +1     
Flag Coverage Ξ”
unit-tests 95.49% <ΓΈ> (-0.02%) ⬇️
unit-tests-14.x-Linux 95.49% <ΓΈ> (ΓΈ)
unit-tests-16.x-Linux 95.49% <ΓΈ> (+0.01%) ⬆️
unit-tests-18.x-Linux 95.49% <ΓΈ> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Ξ”
packages/fast-check/src/arbitrary/unicode.ts 94.44% <0.00%> (-5.56%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tskj
Copy link
Contributor Author

tskj commented Dec 9, 2022

Thanks for the suggestion, I'd definitely go for this one.

Great! I believe this will make users' lives much easier.

However, I still strongly urge that eventually fast-check itself will output counter examples that are copy-pastable. As a very crude approximation, it can even output this very code itself, such as Counterexamples: [[() => fc.sample(fc.context(), {numRuns:1})[0]]] - there really is no reason to ask users to do this by hand!

@dubzzz
Copy link
Owner

dubzzz commented Dec 9, 2022

The PR does not pass, you'll need to run yarn version:bump on it and flag fast-check as a patch version.

Regarding the:

However, I still strongly urge that eventually fast-check itself will output counter examples that are copy-pastable.

As the owner and maintainer of the repository I receive urgency by no-one. The project is made on free time without any funding today. If you really care about it, start funding the project and maybe I'll work on it. But today, it does not pay me at all and I will clearly not spend one second on such a second zone issue that will bring more harm than it fixes.

And once again, the repository being open source, if you get better ideas than someone having worked on the project for more than 5 years now, just fork it or wait.

@tskj
Copy link
Contributor Author

tskj commented Dec 9, 2022

The PR does not pass, you'll need to run yarn version:bump on it and flag fast-check as a patch version.

Ah, my apologies, I was confused by the CONTRIBUTING.md document, it seemed like I wouldn't have to do anything since I didn't do a code change. I have now set it to "decline" since there is no change to the code, let me know if that was wrong or if there is something else I have to do in addition!

As the owner and maintainer of the repository I receive urgency by no-one.

I'm sorry if that is how it came across, I meant to use the phrase "to urge someone" which means "to strongly recommend".

I have no urgency for any fix as I already have my workaround, and I am indeed doing this PR only in the hopes that future people will be able to use this fine library without having to crawl through closed issues.

I do still strongly recommend, and indeed urge you to consider, to take issues with usability seriously. That is a concern of high importance for many development teams around the world, especially those who are careful about the dependencies they want to take on. In that light, having two first class features in your library that do not work together, is a bad look.

This is merely my personal opinion, I am only sharing it because I want the best for the project. What you do with that is up to you, and I respect what ever decision you come to.

@dubzzz dubzzz changed the title Add exampleContext to docs πŸ“ πŸ“ Add exampleContext to docs Dec 9, 2022
@dubzzz dubzzz changed the title πŸ“ Add exampleContext to docs πŸ“ Document how to use context in examples Dec 9, 2022
@dubzzz
Copy link
Owner

dubzzz commented Dec 9, 2022

Seems that formatting is not passing

@tskj
Copy link
Contributor Author

tskj commented Dec 9, 2022

Ah thanks for telling me. Not sure if that was what was breaking it since it didn't give me any error message other than the filename, but this should I work I think. Let me know if there is anything else!

@dubzzz dubzzz merged commit a7ed43a into dubzzz:main Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants