[stp-dev] IRC notes

[12:02] cctrieloff: topics I have so far are
[12:02] *** jrohn has joined #eclipseSTP.
[12:03] cctrieloff: commit process - and related topics
[12:03] cctrieloff: code coverage metrics
[12:03] cctrieloff: close out of the build system
[12:03] *** aem has joined #eclipseSTP.
[12:03] aem: test
[12:03] cctrieloff: missing one topic -
[12:04] *** boisvert has joined #eclipseSTP.
[12:05] cctrieloff: let's start with the commit process mailed out from last weeks meeting
[12:05] DavidBosschaert: Shall I summarize the suggested changes already posted over mail?
[12:05] cctrieloff: yes
[12:05] DavidBosschaert: Okay. I've received the following suggestions:
[12:06] DavidBosschaert: 1. Add a Javadoc requirement
[12:06] DavidBosschaert: 2. Add a CVS branching requirement
[12:06] DavidBosschaert: (We'll leave the coverage to the next agenda item ;)
[12:07] cctrieloff: was that the only two additions?
[12:07] boisvert: if I may interject, I wasn't suggesting that branching was a requirement
[12:07] DavidBosschaert: Okay, let's discuss!
[12:07] boisvert: i was proposion that it would be permissible to commit code in branches before it satisfies all requirements
[12:07] boisvert: (proposing)
[12:08] DavidBosschaert: That sounds fine with me, I guess this touches on the CVS branching strategy that we would use. Does anyone have an opinion on that?
[12:08] cctrieloff: what does TPTP do here?
[12:08] cctrieloff: or DTP?
[12:09] RobCernich: dtp - our requirements are not as stringent
[12:09] DavidBosschaert: Regarding branching, I think this was an option...
[12:09] RobCernich: we'll do branches only to preserve stability going into an iteration or milestone release
[12:10] oisin: how many regular committers on DTP, Rob?
[12:10] cctrieloff: it is not better to keep the mail stable at all times/ if possible
[12:11] aem: Can someone reiterate what we're talking about using branching for here?
[12:11] RobCernich: connectivity: 3-5
[12:11] RobCernich: usually working in their own areas of the code
[12:12] DavidBosschaert: That it would be permissible to commit temp dev branches without adhering to all the quality guidelines.
[12:13] DavidBosschaert: Okay, so does anyone have an issue with this?
[12:13] aem: This is related to the quality guidelines themselves isn't it?
[12:14] aem: What we decide on the next agenda topic about code reviews and coverage may impact whether we need temporary development branches?
[12:14] DavidBosschaert: Yeah, we're talking about the commit process proposal that I mailed to stp-dev earlier this week.
[12:14] DavidBosschaert: Code reviews are in the commit proposal, as well as coverage.
[12:15] cctrieloff: yes, David - summaries what you would suggest for this item, then we can +1 / 0 / -1 and see if we need to discuss more
[12:15] aem: What I mean is that I'm not sure I can comment on our policy for temporary CVS development branches without knowing what we have decided on quality guidelines
[12:15] DavidBosschaert: I would suggest to add to the commit process that to a temp development branch can be committed without satisfying all requirements. Votes?
[12:16] *** RobCernich has signed off IRC (Read error: 104 (Connection reset by peer)).
[12:16] boisvert: +1
[12:16] *** RobCernic1 has joined #eclipseSTP.
[12:16] RobCernic1: sorry, i lost my network connection
[12:16] DavidBosschaert: +1
[12:17] cctrieloff: +1
[12:17] DavidBosschaert: Okay, I will add this suggestion so. None is set in stone. We can always change again.
[12:17] DavidBosschaert: Next topic: Javadoc.
[12:17] cctrieloff: so the sign of of what is committed in the branch in QA'ed before merged to mainline (yes?)
[12:18] aem: How can we decide on whether we can commit temporary CVS branches without deciding what the quality guidelines are?
[12:18] DavidBosschaert: It's really the mainline that counts!
[12:18] DavidBosschaert: BTW we'll talk about the other guidelines in a sec.
[12:18] DavidBosschaert: Alex suggested to at least require some Javadoc on the Class level.
[12:18] DavidBosschaert: Thoughts?
[12:18] boisvert: because whatever quality guidelines we chose, we'll need a way to temporarily suspend those in order to collaborate
[12:18] cctrieloff: basically we have said that we can have committed branched in any state, but they need to be "clean" and meet the quality req before merged to mainline
[12:19] boisvert: yes, that is also my understanding
[12:19] cctrieloff: great (aem) don't know who you are-- that ok?
[12:19] RobCernic1: so, while work is being done, we will always have at least two branches; working and release?
[12:20] aem: That assumes that we force top level quality guidelines.
[12:20] aem: aem = Antony Miguel by the way (sorry, not sure how to set my username)
[12:20] cctrieloff: mainline and then at least 1 branch is how I would put it
[12:20] boisvert: yes, it does; would you like to discuss that?
[12:21] RobCernic1: no, just making sure i understand the process
[12:21] aem: we can discuss that when we discuss code coverage etc, I was just confused as to how we were proceeding
[12:21] boisvert: sorry rob, i was replying to aem
[12:22] cctrieloff: ok - I think we done with this one -David you need to separate the rules for mainline and branches now in the HTML page
[12:22] *** Karlr has joined #eclipseSTP.
[12:22] boisvert: my understanding is that there's a main line of development (aka trunk or HEAD, hopefully stable) and there can be one or more unstable branches
[12:22] DavidBosschaert: Yep, I'll update the html page and will send it out again for comments.
[12:22] cctrieloff: now for mainline commit requirements (JavaDoc and Code coverage)
[12:22] cctrieloff: anyone fire
[12:23] DavidBosschaert: Okay anyone wants to talk about the Javadoc addition?
[12:23] DavidBosschaert: I think it's a good suggestion so I go for +1
[12:23] aem: I think we should not mandate a limit on code coverage or code reviews
[12:23] boisvert: at this point, i am not insisting on javadoc -- i hope people will do but i don't know how to formulate this
[12:23] aem: (sorry, too slow at typing)
[12:24] DavidBosschaert: Okay, so let's make it optional.
[12:24] boisvert: i merely wanted to raise the point so that we make a conscious decision
[12:24] DavidBosschaert: I think it's good practice anyway.
[12:24] RobCernic1: i think there are some requirements in these areas regarding public api definitions
[12:24] DavidBosschaert: So how about this:
[12:25] DavidBosschaert: We strongly suggest that all classes have class-level javadoc and ...
[12:25] DavidBosschaert: also every public method has it.
[12:25] aem: public method or public api method?
[12:25] boisvert: public api method is fine with me
[12:25] aem: I think TPTP and a number of other projects defined public API as anything without a '.internal.' in the package
[12:25] RobCernic1: there's a distinction between public api and public method
[12:26] boisvert: i would also add the suggestion that public api packages also have javadoc (e.g. package.html)
[12:26] boisvert: if only to introduce the package
[12:26] Karlr: I would agree to at least some JavaDoc on all public methods whether they are API or not
[12:26] DavidBosschaert: Good point!
[12:26] RobCernic1: i think the requirements should be less stringent for internal classes
[12:26] boisvert: rob: agreed
[12:26] Karlr: ok that makes sense
[12:27] DavidBosschaert: I agree too.
[12:27] RobCernic1: there are some mandatory requirements for public api's
[12:27] RobCernic1: let me see if i can dig up a link real quick
[12:27] RobCernic1: ties in with jdoc and testing
[12:28] Karlr: http://wiki.eclipse.org/index.php/Javadoc
[12:28] DavidBosschaert: Cool, I'll add all this plus the link to the updated commit.html page
[12:29] Karlr: Looks more like suggestions than it being mandatory though - but good nonetheless
[12:29] cctrieloff: Someone write a summary proposal so all can see and close it with a vote
[12:29] DavidBosschaert: I'll do that and send it over email stp-dev
[12:29] RobCernic1: there are specific requirements that need to be met before an api can be defined as "public"
[12:30] Karlr: That's true, but that's a different issue - important one though
[12:30] DavidBosschaert: Rob, could you elaborate?
[12:30] cctrieloff: We can discuss, but I don't think that goes into a different doc
[12:31] RobCernic1: i'm looking....
[12:31] RobCernic1: all i have is a pdf sent out by our project lead
[12:31] aem: Stuff like all public api must have an example etc as I recall
[12:32] cctrieloff: can you send it to the stp-dev list
[12:32] RobCernic1: yep
[12:32] cctrieloff: then all can look and we pick the discussion up from there
[12:32] cctrieloff: ok - code coverage, the hot topic
[12:33] cctrieloff: wow - I thought all where going to jump in
[12:33] RobCernic1: http://www.eclipse.org/projects/dev_process/eclipse-quality.php
[12:34] cctrieloff: there is not consensus whether or not to enforce 70% coverage for mainline checkins
[12:34] aem: I think we should not make mandatory any coverage limit or code review process
[12:34] RobCernic1: i agree, at least for the early stages of the project
[12:35] RobCernic1: except for api's that may be exposed publicly
[12:35] DavidBosschaert: I think it might actually be easier to start doing it right from the beginning.
[12:35] aem: I don't think we should do it at all
[12:35] cctrieloff: why ?
[12:35] aem: I think it will hamper progress in individual components
[12:36] DavidBosschaert: Nonsense!
[12:36] aem: having a coverage limit and code reviews will work ok if your development methodology is already exactly the same
[12:36] DavidBosschaert: Coverage is automatically computed during test.
[12:36] oisin: looking at the link that Rob posted, one of the quality elements is a test suite - how do we know that a test suite is a valid one?
[12:36] aem: and if you have a number of committers and can do code reviews in house
[12:36] aem: are the tests automatically written?
[12:36] DavidBosschaert: No hand written - automatically run!
[12:37] cctrieloff: let's separate code review and code coverage - they are different
[12:37] aem: right, but what about when we're testing GUIs or distributed engines?
[12:37] DavidBosschaert: Using a Coverage tool is a great way to give you an indication how good your tests are!
[12:37] RobCernic1: note, test suite is a requirement for public api
[12:37] DavidBosschaert: GUI testing can just as well be done with automated tests.
[12:37] *** MikeG has signed off IRC ("Leaving").
[12:37] oisin: aem's point about coverage testing of distributed engines is a fair one I think
[12:37] cctrieloff: so if it is a requirement then should we not measure it
[12:38] RobCernic1: yes, but it is not a requirement for non-api
[12:38] RobCernic1: non-api == project internal
[12:38] DavidBosschaert: But aren't we interested in quality code that works?
[12:38] aem: Writing automated JUnit tests and trying to achieve a 70% coverage rate is just one way to test your code
[12:38] DavidBosschaert: Internal code also needs to be tested because otherwise the product may not work!
[12:39] DavidBosschaert: You can also use system tests for coverage
[12:39] oisin: we are polarizing a bit here can we step back for a second and aem can you expand on your last statement
[12:39] aem: who do you mean oisin?
[12:40] cctrieloff: what I am looking for is stability in the code that we take to mainline, we might have 50 + committers on this project, and it will be impossible for everyone to know all the code. So you need a way to know if you broke something when you take it to mainline
[12:40] RobCernic1: that's a fair statement, but an arbitrary coverage requirement will not guarantee that
[12:40] aem: But do you need to enforce 70% coverage of automated tests to ensure that?
[12:40] oisin: sorry I wasn't clear: Antony, you said something about 'just one way to test code' can you add a bit more to this statement in terms of what you were referring to - is it your position that we shouldn't use junit to test code?
[12:41] DavidBosschaert: Well, if you go much <70%, it means you have a lot of untested code!
[12:41] boisvert: maybe another way to achieve the same thing is to define quality levels for releases, instead of at commit time
[12:41] RobCernic1: untested != poor quality
[12:42] sdaume: absolutely !
[12:42] DavidBosschaert: Tests are best written during development, not afterwards
[12:42] aem: oisin, I just mean that different companies use different testing methods. If you find that mandating a 70% automated test coverage works for you then fine
[12:42] aem: but it won't work for us
[12:42] cctrieloff: let's focus on the goal - we need a way to know in an automated way that the mainline is stable and that a merge has not made all hell break lose
[12:42] aem: David, these are just your opinions, not facts
[12:42] oisin: thanks antony.
[12:42] *** devang has signed off IRC ().
[12:42] boisvert: Rob: most QA people would disagree with you. They would actually say: untested ==> assumed to be broken
[12:42] RobCernic1: :)
[12:43] DavidBosschaert: Right, and the best time to write tests is while developing the actual code...
[12:43] aem: Why not just strongly encourage developers to test their code to make sure it works with the latest build before checking in
[12:43] RobCernic1: i would suggest that tests be written for public apis and apis used internally between projects
[12:43] RobCernic1: i.e. test the boundaries
[12:43] cctrieloff: - so I think we agree that we need to write test for all public API's and that these need to be automated
[12:44] aem: And perhaps also strongly encourage developers to test areas with automated tests where they think the code will benefit
[12:44] aem: I have a public API that takes a BPEL file and runs it on a distributed engine
[12:44] aem: how do I write a JUnit test that checks the thread distribution is correct?
[12:44] boisvert: i tend to agree with carl, i think the goal is to know early if you've broken something
[12:44] DavidBosschaert: I still think that if you don't have tests for your internal code, the product may still be broken.
[12:45] boisvert: the later you find the bug, the more expensive it becomes
[12:45] RobCernic1: testing the edges ensures you haven't broken someone else's code
[12:45] boisvert: and unit tests provide the most effective way to discover bugs early (and often)
[12:45] aem: But do we need to mandate limits?
[12:45] RobCernic1: i don't think so
[12:46] aem: All limits will do is waste some peoples time and allow people with buggy unstable components to hide behind numbers that say they are stable
[12:46] cctrieloff: My suggestion, is that all public API's need a test (even if it takes a BPEL file). Then test would be a BPEL file that if runs in the test harness give a relative assurance that the functionality behind that API is good
[12:47] sdaume: but that might produce a coverage metric ?
[12:47] cctrieloff: basically, the developer need to provide a test - that if passed indicates that
[12:47] sdaume: sorry "might NOT"
[12:47] cctrieloff: that component is goof
[12:47] RobCernic1: i think that's a fair requirement
[12:47] DavidBosschaert: But the quality of that test needs to be measured!
[12:47] cctrieloff: so you would rather have no tests and everyone has to waste there time working out what is wrong
[12:48] oisin: carl, we need an indication if the component is *good* or *goof* :)
[12:48] cctrieloff: :-)
[12:48] aem: I would rather have each component owner that understands their component best decide what the best testing strategy for them is
[12:49] RobCernic1: i think as long as the test exercises the api, to its specification, that should be fine
[12:49] RobCernic1: regardless of how much code that invokes
[12:49] DavidBosschaert: But if it doesn't invoke certain code, that code is dead & should be removed.
[12:49] RobCernic1: i.e. make sure the api functions as specified
[12:50] cctrieloff: and they will provide some automated way from someone else to know that that when they use it it is not broken
[12:50] RobCernic1: that's another topic
[12:50] boisvert: seems like we have two camps here
[12:50] cctrieloff: yip
[12:51] cctrieloff: Do we all agree that we will provide test for public API's +1 /0 /-1
[12:51] oisin: +1
[12:51] boisvert: +1
[12:51] DavidBosschaert: +1
[12:51] cctrieloff: +1
[12:51] aem: automated tests? with or without coverage?
[12:52] boisvert: automated tests, with coverage measurement
[12:52] aem: then -1
[12:52] RobCernic1: +1 automated, where it makes sense, no on the coverage
[12:52] cctrieloff: tests that are can be automated (with 100% coverage of public API ) no internal tests in this vote or internal coverage
[12:52] RobCernic1: +1
[12:53] cctrieloff: i.e. at least 1 test for each public API
[12:53] Karlr: +1
[12:53] oisin: carl: do you mean at least 1 test for each public API _method_
[12:53] cctrieloff: yes
[12:53] aem: What about public GUI widgets?
[12:54] cctrieloff: nope
[12:54] DavidBosschaert: They can also be automatically tested, in general. Esp with Eclipse.
[12:54] cctrieloff: I am breaking the topic up - so that is out for now
[12:54] boisvert: i think one test by public method is going away from our quality goal; i would rather see coverage in terms of % of lines than expressed as testing a method
[12:54] cctrieloff: before we move on --
[12:55] aem: You're creating an exception, and if we're going to have exceptions then why bother making it a hard and fast rule?
[12:55] DavidBosschaert: The 70% figure was to take exceptions into account.
[12:55] cctrieloff: because I am trying to get consensus
[12:56] aem: So a component could be no more than 30% GUI?
[12:56] boisvert: would people agree to lower the % at first, and progressively take it higher for a release?
[12:56] aem: I don't think we should *force* any %
[12:56] RobCernic1: agreed
[12:56] aem: I say lets recommend a testing strategy
[12:56] cctrieloff: no, basically what we are voting on is the model and "engine" we created behind the view of the GUI's and the public API's to them
[12:57] aem: and query committers that have buggy components
[12:57] cctrieloff: aem - make a proposal
[12:58] sdaume: we all have a common goal: high quality
[12:58] sdaume: even 100% coverage does not say anything about quality
[12:58] aem: I propose we strongly recommend that everyone produce automated tests and has a coverage of 70%
[12:58] sdaume: since it does not take into account the quality of the tests
[12:58] aem: But that we leave it up to individual component owners to define their own testing strategy
[12:59] DavidBosschaert: So I guess the difference is in the words insist vs recommend...
[12:59] aem: yes
[12:59] cctrieloff: we all know that this is not perfect no matter what we decide, but I think it is very important that stress quality
[12:59] cctrieloff: I am ok with the wording
[13:00] boisvert: I am +1 for aem's proposal
[13:00] DavidBosschaert: Should we vote on who is in favour of recommend?
[13:00] aem: +1
[13:00] Karlr: +1
[13:00] cctrieloff: as the posting if the defects and defect queue will be a self regulating way for the components measure of quality
[13:00] sdaume: +1
[13:00] RobCernic1: +1
[13:01] DavidBosschaert: 0
[13:01] cctrieloff: so the vote is recommend 70%
[13:01] DavidBosschaert: I will update the commit.html page with this.
[13:01] cctrieloff: does it include each components test strategy (?)
[13:02] boisvert: carl: i don't understand your question
[13:02] cctrieloff: +1 for recommend 70% - want to discuss the each component chose there own strategy
[13:02] aem: if we recommend then we don't mandate any test strategy
[13:02] aem: that's why recommend instead of insist
[13:02] cctrieloff: nope then -1
[13:03] aem: A component could choose not to produce 70% coverage from automated tests
[13:03] cctrieloff: or ?
[13:03] aem: or it could choose to comply with the recommendation and produce 70% coverage from automated tests
[13:04] aem: It's alternative would be to run it's own testing strategy which it believed was more well suited to the individual component
[13:04] *** Karlr has left #eclipseSTP.
[13:04] DavidBosschaert: Whatever the strategy is, we should at least publish the coverage figure.
[13:05] cctrieloff: like what - give me an example (I am not against it, I just don't understand what you might do)
[13:05] RobCernic1: 100% coverage for public API
[13:05] RobCernic1: anything else is bonus
[13:05] aem: I might run distributed BPEL tests over a network of heterogeneous computers and ensure the engine performed properly and didn't deadlock
[13:06] aem: And I'd run them by hand and where I thought it was appropriate to run them because they were quite time consuming
[13:06] cctrieloff: that's great - will the test be check in so someone else can run it?
[13:06] aem: absolutely
[13:06] DavidBosschaert: I am against manual tests. They are a waste of time.
[13:06] aem: In fact they are already there
[13:07] cctrieloff: so your issue is that we might not be able to replicate the test requirements to run it in an automated way
[13:07] DavidBosschaert: Its better to invest some time in automating it.
[13:07] DavidBosschaert: Even distributed tests can be automated.
[13:07] DavidBosschaert: There are open source frameworks for that.
[13:07] aem: or that automated tests will be very hard to write and produce meaningful checks
[13:07] cctrieloff: yes that is true
[13:08] aem: or that they will be too time consuming to write for the benefit we will get from automating them
[13:08] aem: e.g. automated GUI widget tests
[13:08] DavidBosschaert: It will probably cost you more time to manually run them in the end.
[13:08] DavidBosschaert: Read Beck/Gamma's book on 'Contributing to Eclipse' for a large number of examples of automated GUI tests.
[13:09] boisvert: I agree with you David; however my experience has been that for "time-to-market" reasons, you can't always automated everything in time and that you have to live with manual testing for a while until you have the resources/time to automate everything
[13:09] cctrieloff: ok I willing to vote that the test need to be created (required) but the automation does not need to be at the same level -> I know guys like David will get involved and try works out how to get it all automated :-)
[13:09] RobCernic1: i don't see a problem with having manual test procedures
[13:09] aem: and test procedures that don't produce coverage?
[13:09] cctrieloff: I think the state should be :
[13:09] RobCernic1: the goal is quality, not automation
[13:10] aem: RobCernic1
the goal is quality, not automation - exactly
[13:10] DavidBosschaert: Without automation you will never get sustainable quality
[13:11] cctrieloff: Tests are required, automation of these test are highly recommended, and STP goal to eventually have X% of tests automated
[13:11] RobCernic1: x% where feasible
[13:12] cctrieloff: We can up that number as the project matures
[13:12] RobCernic1: i don't see a problem with having certain tests be run manually...
[13:12] aem: Tests are required, automation of these tests is highly recommended and STP's goal is to eventually have X% of the tests automated where feasible
[13:12] RobCernic1: and having those test be run prior to certain releases (e.g. milestone, rcs)
[13:12] aem: vote?
[13:13] cctrieloff: yes - so we the component owner needs to run the non automated test before milestones, rcs etc
[13:13] cctrieloff: +1
[13:13] aem: +1
[13:13] RobCernic1: +1
[13:13] sdaume: +1
[13:14] DavidBosschaert: I think we could say that code really *should* have automated tests,
[13:14] DavidBosschaert: if its absolutely necessary to have manual ones they should be run before milestone =1
[13:14] DavidBosschaert: +1
[13:14] cctrieloff: should is a better word - yes to your comment
[13:15] cctrieloff: oisin?
[13:15] oisin: +1
[13:15] oisin: that looks like a consensus
[13:15] oisin: :)
[13:15] cctrieloff: yip
[13:16] cctrieloff: ok - David can you update the HTML, we will post that as v1
[13:16] cctrieloff: and CVS is officially open then :-)
[13:16] cctrieloff: agree?
[13:16] DavidBosschaert: Okay, will do.
[13:17] oisin: smash the bottle of bubbly on it and off we go then...
[13:17] cctrieloff: :-)
[13:17] aem: thanks everyone, glad we could reach an agreement
[13:17] cctrieloff: There was one last request to have the IRC an hour earlier
[13:17] cctrieloff: vote (+1 / 0 /-1)
[13:17] aem: ok by me
[13:17] aem: +1
[13:17] sdaume: +1
[13:17] *** aem has left #eclipseSTP.
[13:17] oisin: -1 clash with SCA call
[13:17] boisvert: -1
[13:18] RobCernic1: 0
[13:18] DavidBosschaert: (I think another day was also suggested)
[13:18] cctrieloff: that went down on the list
[13:18] cctrieloff: we keep the time then until a suggest passed vote (feel free to try one on the dev list)
[13:18] cctrieloff: thanks all
[13:19] oisin: thanks carl