Breaking change detection about semantic-release HOT 7 CLOSED

this sounds pretty awesome! 👍

from semantic-release.

@boennemann that's a really cool idea. Keep in mind that it will report a lot of false positives (eg. if my unit tests test a method's implementation, not interface), and will fail to report a lot of stuff (eg. if a test didn't test the right thing, or if an argument type change still makes tests pass).

i took a very different approach with india, relying on jsdoc instead: https://github.com/bcherny/india. Your way of doing it is less fragile than my approach, I think, but may be less accurate.

from semantic-release.

@bcherny Thank you very much for this comment!

False negatives:
I was thinking about this stuff and what it comes down to is that whenever this method fails your test setup isn't optimal. I know that this happens a lot, but I'm not trying to fix these testing problems, but I'm trying to achieve automation for software releases that is better, or at least on par, with what the human would decide. I doubt you would recognize these breaking changes by hand, if your tests are that insufficient.
The way to make the automation better is to stick to best practices and that's a pretty cool thing actually.

False positives:
Same as above: in your example the tests are the problem. There are probably some scenarios where false positives would still be reported. We should add an option to pass the "breaking change detection" for these cases.

I had a brief look at india, and I really like this as well. I would be more than happy if we could build both strategies, or even a combination into this.

Also I should probably make the breaking-change detection its own module.

from semantic-release.

I think it's more than the test setup being "unoptimal". There are different ways of unit testing stuff, and depending on what you're testing it's reasonable to use the approach that's the best fit. Some examples of this are:

• Fully sandboxed unit testing (mocking out all dependencies, Jest-style)
• Sometimes-sandboxed unit testing (mocking out some dependencies)
• Never-sandboxed unit testing (driving partial-stack integration tests with unit tests)
• Testing implementations (eg. #foo called #bar with "bar", example)
• Testing state (example)
• Testing interfaces (example)
• Testing DOM-related stuff with a detached DOM (example)
• etc.

With good test coverage, it's typical to have some mix of the above in your tests (not just interface tests). Interface tests are good for supplementing JavaScript's lack of static typing, but other types of tests are useful for other things.

False negatives can potentially be an issue too. For example, say I have this code:

// source
function multiply (a, b) {
  return a * b
}

// test
assert(multiply(2,2) === 4)

I then decide that multiply should also accept Strings, so I add some tests:

assert(multiply('2',2) === 4)
assert(multiply('2','2') === 4)

Because multiply doesn't typecheck its arguments, it ends up being more permissive than it ought to be. An automated semver tool would mark this as a patch change, but really it's a minor change because you went from accepting arguments of type Number to arguments of type String|Number (as specified in the tests).

I'm worried that there are too many edge cases like these to make an automated semver tool based on unit tests useful in large projects. On the flip side, my approach with India creates the risk that jsdoc annotations will grow out of sync with function signatures, and makes maintaining your jsdoc necessary. Though Google Closure has enforced type annotations for years (in service of compiling code more efficiently), and Facebook's Flow does as well.

What do you think?

from semantic-release.

@bcherny Thanks again for the comment. I really appreciate the discussion here :)

I think you are raising very valid concerns.
Probably the breaking change detection should never automatically cause a major version bump, but only raise an error that one can suppress with explicitly marking the commit as non-breaking.

That said I would wan't to implement this in a "plugin" kind of way (see #9) and I think both india and my approach would make for a great plugin for semantic-release. In the end it comes down to individual preferences and I would be happy to fulfill any of these with a modular approach :)

from semantic-release.

(I didn't read the discussion completely, so excuse if i have duplicate input)
Only updating the test folder assumes that all tests in the test folder test only public API. This means that way of detecting breaking changes strongly depends on the way testing was done in this project. Quickly the mockist vs. classisist discussion comes to my mind, or unit and BDD style testing. They all have different implications.
Maybe a "new" category of tests can be defined, kinda like "API tests", "public tests" or maybe even integration tests ... not sure. And those can be used to test what you described. But still that means it depends on people doing it right in this category.

from semantic-release.

https://github.com/christophwitzko/cracks

from semantic-release.