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

Hypothesis #1378

Closed
benjamin-kirkbride opened this issue Jul 12, 2023 · 5 comments
Closed

Hypothesis #1378

benjamin-kirkbride opened this issue Jul 12, 2023 · 5 comments

Comments

@benjamin-kirkbride
Copy link
Contributor

benjamin-kirkbride commented Jul 12, 2023

On a more broad note, how would you feel about adding https://hypothesis.readthedocs.io/en/latest/ to the test suite?

Originally posted by @benjamin-kirkbride in #1328 (comment)

I would like to use Hypothesis as part of the test suite. @Akuli originally said he was not a fan of this idea. I am hoping that he is open to having his mind changed on the topic :)

The specific request here is similar to when I asked about #1332, are you open to the idea such that a PR would be looked at and considered?

My strategy and rationale for Hypothesis is:

  • Incremental addition: Hypothesis is not intended to replace existing concrete tests, but to complement them. By incorporating generative testing, we can catch unexpected issues and enhance the robustness of our test suite.
  • Scalability: Property-based tests with Hypothesis provide a concise and scalable approach to testing, reducing maintenance by expressing intended behavior at a higher level rather than writing individual test cases for each scenario. I think this will be especially useful as we re-work a lot of functionality into actions; we can write "contracts" as to what the contracts should be able to handle, and trust that all of the consumers will "just work". It will also prevent me from making 1000 tests for a regex, as I am otherwise inclined to do ;)
  • Bug prevention: Hypothesis helps uncover edge case-related bugs that may not be immediately apparent during development or in real-world usage scenarios. It safeguards against unexpected inputs and improves software stability. Given the nature of the project; literally the whole point is accepting arbitrary input, both from the top-down from user inputs (mouse kb etc) and bottom up (files, network, etc).
  • Clear test output: Despite generating numerous test cases, Hypothesis offers informative and concise output. It highlights failing examples, facilitating quick bug identification and debugging.
@benjamin-kirkbride
Copy link
Contributor Author

@Akuli had this to say:

I'm not a big fan of hypothesis. IMO tests should be as concrete/specific as possible, such as ListItemCase(id="numbered 42", line="42) item 1", expected=True), whereas the code being tested tends to be written very generally. This ensures that at least the test is correct, because it's ideally very simple.

And yes, I know that the point of hypothesis is to catch bugs where a specific string that you didn't think of causes a problem. But they tend to get caught without unit tests in Porcupine. For example, the empty line.splitlines() bug caused errors when I opened a file.

I can see hypothesis being useful in other projects though. For example, maybe you have functions to escape and unescape a string in some way, and you want to know that they work in all corner cases.

@benjamin-kirkbride benjamin-kirkbride changed the title > We really don't need more than a thousand green dots when running pytest because of one regex Hypothesis Jul 12, 2023
@Akuli
Copy link
Owner

Akuli commented Jul 12, 2023

I don't really need to know more about why hypothesis helps in general. I know the arguments for it, and I don't feel like I need to read the same things about it one more time. To convince me, you instead need to provide concrete examples about how Porcupine benefits from it.

For example, let's say you make a pull request where a test would be simpler/better written with hypothesis than without. Just use hypothesis. I will complain in the review if I think there is a better way to write the test. The same goes for all other tricks: just use the tool that is best for the job, and if I don't agree, I will point out in the review.

I think this issue can be closed, because there's nothing actionable to be done here, unless you want to use hypothesis to find bugs from some existing functionality?

@benjamin-kirkbride
Copy link
Contributor Author

benjamin-kirkbride commented Jul 12, 2023

Just use hypothesis
The problem is these tests would error out and not work locally for you unless you installed Hypothesis. I felt like I needed more explicit approval that you are open to that before doing this, which it sounds like are are giving me here.

there's nothing actionable to be done here

The specific request here is similar to when I asked about #1332, are you open to the idea such that a PR would be looked at and considered?

You answered this question, so this can probably be closed, yes.

unless you want to use hypothesis to find bugs from some existing functionality?

I do want to do this, but probably not in the immediate term

@Akuli
Copy link
Owner

Akuli commented Jul 12, 2023

I felt like I needed more explicit approval that you are open to that before doing this, which it sounds like are are giving me here.

In my projects, there's nothing wrong with adding something questionable into a pull request, even if it's a new dependency :)

@benjamin-kirkbride
Copy link
Contributor Author

Thanks for the clarification :)

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

No branches or pull requests

2 participants