vila, jfunk: http://paste.ubuntu.com/5619580/ that's a test that fails without my change, and that doesn't use the browser so doesn't need a real or faked SSO. probably, it would look nicer starting a patcher for all the mocks. I was just trying to do it in 20 minutes :) does it suffice for you? elopio: urhg, mocks all over the place :-/ And you'll tell me that it will be tested against real code somewhere else ? vila: on pay. With a real browser and a real server. that's a clear demonstration that mocked tests allows one to write complicated tests (it's really hard to read and requires an almost complete knowledge of the implementation to understand) that's what I told you in the meeting. so instead of helping sharing the knowledge, it *requires* the knowledge the only way to avoid the fakes was to mock everything. I probably missed when you said 'mock' then elopio: every time someone tells me "that's the only way", I know they're wrong. In software there is never a single way to do something this one is cleaner: http://paste.ubuntu.com/5619597/ vila: if you don't mock those actions, you need a real or faked browser. Or mock the actions on sst. mock the actions on sst would require even a deeper knowledge of the implementation. elopio: I think I was clear about wanting a fake... the fake *describes* the knowledge and the requirements for the test in this case that's the exact opposite of what you're doing here just as the test_register_new_user in pay does, it describes the knowledge. that's why I don't want to maintain another test or a fake. elopio: looks like you don't get what I call knowledge here :-( elopio: either your helper can be defined without links to external apps and you should be able to test it, or it can't and it needs to designed differently s/to designed/to be designed/ that's what I was advocating during the meeting <-> nessita is now known as nessita-teaching vila: maybe you can start proposing alternative designs. I'm currently writing the test you want. that will take a bit longer. but the problem it's not writing it. The problem is writing it for all the helpers. elopio: the first test is the hardest to write, focus on that one, we'll see for the others later (I didn't realize they were missing :-/) vila: it would be just as hard everytime you have to fake a page. They are not hard to write, it's easy, take a look at the test_page.py the problem is maintaining them. With little values in my opinion, as I consider them duplicated. elopio: I'm afraid you won't be able to get to the point where each project tests what it should if you start with the idea that you will end up with duplication elopio: instead, try to find how to reach the point where each project only has to check what it cares about elopio: and what matters about test failures (excluding the flaky ones) is not that you have to fix them but that they are trivial to fix when you do trivial changes. The value there is to get confidence that your tests catch changes in your code. The flip side being that if your change does not break other tests, you didn't break the other features either. elopio: just today, I made a change that broke ~15 of the 40 tests I added. All failures were easy to fix and *valid*. Once I fixed them, I didn't have to think about whether my change was valid. The added tests were enough for that. vila: still around? vila, jfunk: http://paste.ubuntu.com/5619829/ I'm going to send an email, as vila is EOD and I'm on holidays tomorrow. elopio: was eating, the paste looks.... just right :) And since it's the first test, it's likely we can find ways to make the tests even simpler vila: I'm in the middle of the email. You will find there that I'm not complaining because it was hard. elopio: during dinner, I wondered about an alternate design where we fake sst (that would make the tests faster), but I see you're using file:/// which means you're testing more but that's ok there's nothing there we haven't solved before elopio: even better then elopio: why didn't you start with that then ? vila: because in my opinion that's a duplicate test that adds no big value. I'm making a summary on the email... almost done. elopio: the pages you're defining here are a simplification of the real ones, containing only what is required. elopio: that has value, it describes the knowledge used to handle the redirect without all the noise in the real pages vila: I'm not saying it doesn't have value. A small value, in my opinion. elopio: if you don't see this value... you're missing why tests should be focused elopio: if I have to explain some detail about an implementation, I don't throw a whole tree of sources at you, I point you to a test elopio: I don't even have to go into wordy explanations elopio: and that explanation will stay valid or be updated because the test will fail otherwise elopio: contrast with your mock implementation where there is not a single trace of what matters in the real pages elopio: and think about having a common base for your page variants, defining the relevant bits in the base class and having only implementation details in the daughter classes (real server or string page) elopio: still not seeing the value ? I see value, not big. The base page class is fully covered by tests in u1testutils. the implementation details of the real pages are what the acceptance tests check. if you want another test for the implementation details, then, IMO, it's a duplicate test. then the helpers cannot fail when doing so hence they need to be tested independently no matter how you look at the problem, you can't avoid that elopio: please, stop telling *I* want duplicate tests, I don't. elopio: if *you* end up with duplicate tests, you missed something either in what I said or in how you do it I'm not saying you want duplicate tests. I'm saying I consider the tests you want to be duplicates, there's a slight difference. well, look at it this way: in this specific case, there aren't duplicate because the helper is not used against the same pages the string pages do not require a server, that make it easier to write tests if from these tests, I'm confident enough in my code, I won't need to directly test these helpers against a server I can just use them into other tests without having to worry about errors there if I'm allowed to run more tests then I can *also* run the same kind of tests against a real server to be really sure, say, once a day/week/month but *that* will only have value if failures occur (which will likely indicate *other* problems so still, not a big value) vila: I understand your reasons, I'm listening to you, please don't think I'm just ignoring that. still, I prefer not doing them. vila, jfunk: ok, mail sent. All my thoughts are summarized there. elopio, thx elopio: then I don't get why you insisted so much to get my agreement when the only thing I did was to voice my concerns in your proposal. I didn't vote Disapprove there but Needs Information elopio, did you mean to paste the same link 2x in the email? vila: because I can't merge the branch with your needs information. jfunk: probably not. checking. jfunk: yes, the right link is http://paste.ubuntu.com/5619829/ elopio: so, instead of finding another reviewer you wanted me to forget about my concerns ? vila: no, it doesn't matter how many reviewers I get, tarmac won't land the branch as long as there's one needs information. urgh, fix tarmac :) vila: I like tarmac, I like the process it enforces. elopio: voted Abstain vila: I provided you with all the information you requested, so, yes, abstain or reject sound better. elopio: well, I didn't realize needsInfo could block a proposal, sounds.... weird to say the least so, now that you are abstain, I need another reviewer to approve or reject. There's where jfunk comes. just reviewing everything now elopio, vila - I'm leaning strongly towards Leo's recommendation jfunk: did you read all that vila has said here in the last hour? elopio, yes I have ok, then I'm happy with that, of course. elopio, though I wish there was some way to write the 'fake' tests and reuse them for the ones you're proposing so they only needed maintenance in one place, because the simplicity and confidence they allow is a real bonus jfunk: I agree with the last part, I don't know how to implement them with maintenance in one place. or maybe I misunderstood. do you have an idea about how to do that? not at the moment jfunk: ok, so can you vote here, please? https://code.launchpad.net/~elopio/u1-test-utils/log_in_from_redirect/+merge/160537 and, it really sounds impressive, but I'm not yet tired of discussing about this :) feel free to throw more ideas to the solution pond. elopio, approved thanks. vila, jfunk, sorry for taking so much of your time. now... profit! I'm more worried about time lost debugging without proper tests.. vila, I considered the debugging time lost - I hate to let you down :( but given the current attitude in dev right now toward maintaining our tests makes it very important to keep the barrier of test writing low - in the future we will revisit this topic