4 ms·

This seems like not a big deal. It’s a common mistake and is in 5% of repos but it’s not causing major damage. And there’s no evaluation of importance as to wh

This seems like not a big deal. It’s a common mistake and is in 5% of repos but it’s not causing major damage. And there’s no evaluation of importance as to whether these instances are in test files or non-critical code. Packages are big and can have hundreds or thousands of files. It could be that if these mattered, they would have been detected and fixed. A good example for unit tests and perhaps checking to see if these bugs are covered or not covered. I like these kinds of analyses but don’t like the presented like it’s some significant failure.

jollybean 5y ago
5% of 'released' software is quite a lot, more importantly it's a class of errors that definitely should not exist. This is a 'bug' in the language effectively there just isn't any real upside. Python has a few of these things, which is really sad.
- bcrl 5y ago
  It's a class of error that would be caught by even the most basic testing. A better title for the article is that 5% of 666 Python repos have typos that demonstrate the code in them that is completely untested. It doesn't matter which language it is: untested code is untested code in any language.
  - rikatee 5y ago
    unfortunately like 10% of the bugs were in the tests themselves. e.g., the sentry one https://codereviewdoctor.medium.com/5-of-666-python-repos-had-comma-typos-including-tensorflow-and-pytorch-sentry-and-v8-7bc3ad9a1bb7#15b3 https://codereviewdoctor.medium.com/5-of-666-python-repos-ha... the tests are only as good as the code they're written with, and as good as the code review process they were merged under.
    - bcrl 5y ago
      One of the habits I have when writing kernel code is to intentionally break code in the kernel to verify that my test is checking what I think it's checking. That's because of a lesson I learned a long, long time ago after someone reviewed my code and caught a problem: when your code has security implications, you need to make sure the boundary conditions that your tests are supposed to cover actually get tested. Having implemented a number of syscalls exposted to untrusted userland over the years, this habit has saved my bacon several times and avoided CVEs.
    - wott 5y ago
      I believe that, whenever possible, tests should be written in a different language that the one used for the code under test (even better, in a dedicated, mostly declarative, testing language). It avoids replicating the same category of errors in both the test and the code under test, especially when some calculation or some sub-tests generation is made in the test.
  - geofft 5y ago
    The errors were usually in tests themselves. Are you arguing that tests need their own tests to test that they are testing the right thing? Usually I think people believe that tests do not need to be tested and should not be tested, i.e., that you measure "100% coverage" against non-test code alone.
    - samhw 5y ago
      I don't think anyone could disagree: you could never exceed 0% code coverage if your definition was recursive (i.e. included tests, tests-of-tests, tests-of-tests-of-tests, ...).
      - Cpoll 5y ago
        Only if you generate infinite tests, then your coverage approaches 0%. But 100% covered code + 0% covered tests = ~50% total coverage. Also, the obvious solution is self-testing code. (Jokes aside, structures like code contracts attempt something like this).
  - jollybean 5y ago
    "It's a class of error that would be caught by even the most basic testing. " You could say that about anything and everything in software. It's not acceptable that testing needs to be run for something the language should 100% accommodate. The whole point of the language is to provide algorithmic clarity and avoid these things. This isn't really an issue of 'trade offs' is just a bad feature of the language that should have been remedied more than a decade ago. The lack of proper declaration of variables is even more absurd, there's only downside to that.
  - deleted 5y ago
    [deleted]
- jve 5y ago
  I checked those those 11 links to issues for major software. 10 bugs were actually in tests...
  - oaiey 5y ago
    I do not see this from a verification perspective ... But also from a productivity perspective.
  - ErikCorry 5y ago
    This is understandable since many of those projects are not written in python. So the python code in them is only in incidental scripts like test harnesses. If V8 was written in python then performance would probably not be very good.
  - deathanatos 5y ago
    9 out of 10, actually; the Tensorflow links are the same link.
- onphonenow 5y ago
  There were proposals to fix some of these but the unicode zeal beat out some of the more boring (but I'd say as important) cleanups.
enchiridion 5y ago
I mean, if you’re ultimately going to combine the list into a string anyway it’s no big deal. Along those lines. I wonder how many of these come from ad-hoc file path handling instead of using pathlib.
rikatee 5y ago
yeah the impact varies. the sentry one seems pretty big: https://codereviewdoctor.medium.com/5-of-666-python-repos-had-comma-typos-including-tensorflow-and-pytorch-sentry-and-v8-7bc3ad9a1bb7#15b3 https://codereviewdoctor.medium.com/5-of-666-python-repos-ha... test did not work but did not fail either, imagine being that dev maintaining the code that the test professes to cover. Imagine being the user relying on the feature that test was meant to check (if the feature under test actually broke).

jollybean 5y ago

5% of 'released' software is quite a lot, more importantly it's a class of errors that definitely should not exist. This is a 'bug' in the language effectively there just isn't any real upside. Python has a few of these things, which is really sad.

bcrl 5y ago
It's a class of error that would be caught by even the most basic testing. A better title for the article is that 5% of 666 Python repos have typos that demonstrate the code in them that is completely untested. It doesn't matter which language it is: untested code is untested code in any language.
- rikatee 5y ago
  unfortunately like 10% of the bugs were in the tests themselves. e.g., the sentry one https://codereviewdoctor.medium.com/5-of-666-python-repos-had-comma-typos-including-tensorflow-and-pytorch-sentry-and-v8-7bc3ad9a1bb7#15b3 https://codereviewdoctor.medium.com/5-of-666-python-repos-ha... the tests are only as good as the code they're written with, and as good as the code review process they were merged under.
  - bcrl 5y ago
    One of the habits I have when writing kernel code is to intentionally break code in the kernel to verify that my test is checking what I think it's checking. That's because of a lesson I learned a long, long time ago after someone reviewed my code and caught a problem: when your code has security implications, you need to make sure the boundary conditions that your tests are supposed to cover actually get tested. Having implemented a number of syscalls exposted to untrusted userland over the years, this habit has saved my bacon several times and avoided CVEs.
  - wott 5y ago
    I believe that, whenever possible, tests should be written in a different language that the one used for the code under test (even better, in a dedicated, mostly declarative, testing language). It avoids replicating the same category of errors in both the test and the code under test, especially when some calculation or some sub-tests generation is made in the test.
- geofft 5y ago
  The errors were usually in tests themselves. Are you arguing that tests need their own tests to test that they are testing the right thing? Usually I think people believe that tests do not need to be tested and should not be tested, i.e., that you measure "100% coverage" against non-test code alone.
  - samhw 5y ago
    I don't think anyone could disagree: you could never exceed 0% code coverage if your definition was recursive (i.e. included tests, tests-of-tests, tests-of-tests-of-tests, ...).
    - Cpoll 5y ago
      Only if you generate infinite tests, then your coverage approaches 0%. But 100% covered code + 0% covered tests = ~50% total coverage. Also, the obvious solution is self-testing code. (Jokes aside, structures like code contracts attempt something like this).
- jollybean 5y ago
  "It's a class of error that would be caught by even the most basic testing. " You could say that about anything and everything in software. It's not acceptable that testing needs to be run for something the language should 100% accommodate. The whole point of the language is to provide algorithmic clarity and avoid these things. This isn't really an issue of 'trade offs' is just a bad feature of the language that should have been remedied more than a decade ago. The lack of proper declaration of variables is even more absurd, there's only downside to that.
- deleted 5y ago
  [deleted]
jve 5y ago
I checked those those 11 links to issues for major software. 10 bugs were actually in tests...
- oaiey 5y ago
  I do not see this from a verification perspective ... But also from a productivity perspective.
- ErikCorry 5y ago
  This is understandable since many of those projects are not written in python. So the python code in them is only in incidental scripts like test harnesses. If V8 was written in python then performance would probably not be very good.
- deathanatos 5y ago
  9 out of 10, actually; the Tensorflow links are the same link.
onphonenow 5y ago
There were proposals to fix some of these but the unicode zeal beat out some of the more boring (but I'd say as important) cleanups.

enchiridion 5y ago

I mean, if you’re ultimately going to combine the list into a string anyway it’s no big deal. Along those lines. I wonder how many of these come from ad-hoc file path handling instead of using pathlib.

rikatee 5y ago

yeah the impact varies. the sentry one seems pretty big: https://codereviewdoctor.medium.com/5-of-666-python-repos-had-comma-typos-including-tensorflow-and-pytorch-sentry-and-v8-7bc3ad9a1bb7#15b3 https://codereviewdoctor.medium.com/5-of-666-python-repos-ha... test did not work but did not fail either, imagine being that dev maintaining the code that the test professes to cover. Imagine being the user relying on the feature that test was meant to check (if the feature under test actually broke).