QEP 17: Required unit testing for "critical" classes
We need to decide if we want to go ahead with QEP 17 which addresses mandatory testing of critical classes.
https://github.com/qgis/QGIS-Enhancement-Proposals/issues/30
Please keep the discussion related to this proposal in the comments for the QEP page itself.
Poll Created Wed 4 Nov 2015 8:49PM
We should require mandatory tests of core classes as per QEP #17 Closed Sat 7 Nov 2015 8:07PM
If you agree with the proposal as laid out in the QEP, please approve this.
Results
Results | Option | % of points | Voters | |
---|---|---|---|---|
|
Agree | 50.0% | 3 | |
Abstain | 50.0% | 3 | ||
Disagree | 0.0% | 0 | ||
Block | 0.0% | 0 | ||
Undecided | 0% | 4 |
6 of 10 people have participated (60%)
Marco Hugentobler
Thu 5 Nov 2015 7:13AM
I don't mind. But if a PR is rejected because of missing unit tests, the dev should really get a meaningfull error message. Otherwise, it will prevent people from bringing their modifications into master branch.
Paolo Cavallini Thu 5 Nov 2015 2:48PM
Still uncertain. I think we should implement this gradualy, and adapt to what the outcome is.
Jürgen E. Fischer Fri 6 Nov 2015 5:18PM
Generally I'm in favor of this. But the passage "and commits pushed to master which also violate this will be reverted." is a nonono for me. Missing tests could probably be easily added with another commit - and reverting the commit in question IMHO isn't necessary.
I think that this will probably (/hopefully) never happen in practice anyway and resolved by giving a dev in question a little nudge instead. But this is all about strict rules, so we would avoid rules that we don't actually follow.
Tim Sutton Sat 7 Nov 2015 8:15AM
@nyall can I suggest you change the wording to make it clear that we will treat exceptions by their merits rather than making this a rigorously enforced rule.
Andreas Neumann · Thu 5 Nov 2015 7:51AM
If it helps to maintain/raise quality - I will support this QEP. Devs should be supportive, if a PR concerning core classes misses unit tests and the dev is not so familiar with unit tests. At least in the transition phase.