Re: Android JUnit Tests Now Pass
Awesomesauce. Going to move forward then (with putting back the accidentally deleted test). If there's other things missed, they can be brought back as well. On Thu, Feb 12, 2015 at 12:47 PM, Brian LeRoux b...@brian.io wrote: I see no situation where we don't want a feature branch vetted by 1 person before we land anything on master …short of fixing broken tests. I assume good faith. Joe: you had a bad day and, I think, you still feel mistrust after previous commits landing on master stalling out your work last summer. Lets put that behind us. Andrew pls fire a ping to the list w/ a PR for anything aiming to live on Android master until earn Joe's trust back. And lets avoid the editorial about motivations. We all want the same thing here: work on a kick ass open source project that makes a difference. On Thu, Feb 12, 2015 at 9:31 AM, Jesse purplecabb...@gmail.com wrote: This commit may not have warranted this discussion. I think we agree that large changes/commits should be on feature branches, and discussed before being merged. Let's go with that. On Feb 12, 2015, at 8:49 AM, Andrew Grieve agri...@chromium.org wrote: Sounds like you've been having a rough time. :( Hope you get through it. Believe me when I say I hear you loud and clear about making changes on feature branches. I just don't think this one fits. - No one (other than me) has touched the tests since September of last year, so it's unlikely that a change would cause merge conflicts. - The state of the tests show that they are not viewed as that important (at least not important enough for anyone other than me to have been working on them) - Anything I do to them doesn't affect shipping code. No risk. I find it hard to believe that my making changes contributes in a significant way to having people avoid working on Android. Perhaps being overly abrasive via email JIRA would be a deterrent though... On Thu, Feb 12, 2015 at 11:10 AM, Joe Bowser bows...@gmail.com wrote: On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve agri...@chromium.org wrote: I agree that significant changes should be reviewed first. But for the most part Cordova is a review-after-commit kind of place, No, it's not. Cordova is only like that because you consistently make it like that. Constantly committing to master without any review at all makes it next to impossible for anyone else to work on the project. You can tell that this is the case, because you own the majority of the commits over the last few months. That's not normal for a codebase this size with this many contributors. This is why we have topic branches, and we've had this discussion with you numerous times about using them. This is also why I write e-mails trying to get buy-in to what I want to do instead of just landing features straight on master that could break everything. and this change didn't touch any code that we release (strictly tests... that have been broken for a very long time), so I don't think it qualifies. I'll admit that the tests were a bit of the wild west. That said, there was always an understanding that tests would be added to and improved upon and not removed. Marcel and I probably wouldn't have had half the e-mails that we have had if it wasn't us arguing over whether to delete tests. I know it's frustrating to have to wait on other people, since people are human, get sick, and have to take care of others when they're sick. That said, it's equally frustrating to come back from vacation, or wake up from a nap after driving someone from the hospital and see that critical code that was a major issue only six months ago got accidentally removed in a sweeping change, along with other use cases. That's what happened yesterday, and that's why I got frustrated. If I'm having a bad day already, a random refactor that just gets dropped without at least a head's up beforehand makes it worse. I've been on both sides of the issue with this. I remember getting extremely frustrated with Bryce when we designed CordovaWebView, especially since my design had less of a change to the code. I thought things were moving too slowly, but at the end of the day we did produce something that a lot of people seem to use (at least that's what the sample repo I have on GitHub tells me, the GitHub analytics tools are all I have to go on). That said, we didn't ship that until it was mostly ready, and other than an awkward presentation at PhoneGap Day, it was mostly fine. I'm glad I didn't just merge my crap in and just unilaterally introduce that feature, since back then we could still get away with that technically. But yeah, can we have things on feature branches if they're that big, and then wait maybe 24 hours
Re: Android JUnit Tests Now Pass
You may or may not, but I think it would be nice to let others review your (significant) changes before dumping them to master. On Feb 11, 2015, at 6:34 PM, Andrew Grieve agri...@chromium.org wrote: On Wed, Feb 11, 2015 at 5:00 PM, Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. Bit of a jump to conclusions, don't you think? What makes you think I don't recognize the code I changed? @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
Re: Android JUnit Tests Now Pass
on a branch? ;) On Thu, Feb 12, 2015 at 2:47 PM, Andrew Grieve agri...@chromium.org wrote: Awesomesauce. Going to move forward then (with putting back the accidentally deleted test). If there's other things missed, they can be brought back as well. On Thu, Feb 12, 2015 at 12:47 PM, Brian LeRoux b...@brian.io wrote: I see no situation where we don't want a feature branch vetted by 1 person before we land anything on master …short of fixing broken tests. I assume good faith. Joe: you had a bad day and, I think, you still feel mistrust after previous commits landing on master stalling out your work last summer. Lets put that behind us. Andrew pls fire a ping to the list w/ a PR for anything aiming to live on Android master until earn Joe's trust back. And lets avoid the editorial about motivations. We all want the same thing here: work on a kick ass open source project that makes a difference. On Thu, Feb 12, 2015 at 9:31 AM, Jesse purplecabb...@gmail.com wrote: This commit may not have warranted this discussion. I think we agree that large changes/commits should be on feature branches, and discussed before being merged. Let's go with that. On Feb 12, 2015, at 8:49 AM, Andrew Grieve agri...@chromium.org wrote: Sounds like you've been having a rough time. :( Hope you get through it. Believe me when I say I hear you loud and clear about making changes on feature branches. I just don't think this one fits. - No one (other than me) has touched the tests since September of last year, so it's unlikely that a change would cause merge conflicts. - The state of the tests show that they are not viewed as that important (at least not important enough for anyone other than me to have been working on them) - Anything I do to them doesn't affect shipping code. No risk. I find it hard to believe that my making changes contributes in a significant way to having people avoid working on Android. Perhaps being overly abrasive via email JIRA would be a deterrent though... On Thu, Feb 12, 2015 at 11:10 AM, Joe Bowser bows...@gmail.com wrote: On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve agri...@chromium.org wrote: I agree that significant changes should be reviewed first. But for the most part Cordova is a review-after-commit kind of place, No, it's not. Cordova is only like that because you consistently make it like that. Constantly committing to master without any review at all makes it next to impossible for anyone else to work on the project. You can tell that this is the case, because you own the majority of the commits over the last few months. That's not normal for a codebase this size with this many contributors. This is why we have topic branches, and we've had this discussion with you numerous times about using them. This is also why I write e-mails trying to get buy-in to what I want to do instead of just landing features straight on master that could break everything. and this change didn't touch any code that we release (strictly tests... that have been broken for a very long time), so I don't think it qualifies. I'll admit that the tests were a bit of the wild west. That said, there was always an understanding that tests would be added to and improved upon and not removed. Marcel and I probably wouldn't have had half the e-mails that we have had if it wasn't us arguing over whether to delete tests. I know it's frustrating to have to wait on other people, since people are human, get sick, and have to take care of others when they're sick. That said, it's equally frustrating to come back from vacation, or wake up from a nap after driving someone from the hospital and see that critical code that was a major issue only six months ago got accidentally removed in a sweeping change, along with other use cases. That's what happened yesterday, and that's why I got frustrated. If I'm having a bad day already, a random refactor that just gets dropped without at least a head's up beforehand makes it worse. I've been on both sides of the issue with this. I remember getting extremely frustrated with Bryce when we designed CordovaWebView, especially since my design had less of a change to the code. I thought things were moving too slowly, but at the end of the day we did produce something that a lot of people seem to use (at least that's what the sample repo I have on GitHub tells me, the GitHub analytics tools are all I have to go on). That said, we didn't ship that until it was mostly ready, and other than an awkward presentation at PhoneGap Day, it was mostly fine. I'm glad I didn't just
Re: Android JUnit Tests Now Pass
I agree that significant changes should be reviewed first. But for the most part Cordova is a review-after-commit kind of place, and this change didn't touch any code that we release (strictly tests... that have been broken for a very long time), so I don't think it qualifies. On Thu, Feb 12, 2015 at 4:07 AM, Jesse purplecabb...@gmail.com wrote: You may or may not, but I think it would be nice to let others review your (significant) changes before dumping them to master. On Feb 11, 2015, at 6:34 PM, Andrew Grieve agri...@chromium.org wrote: On Wed, Feb 11, 2015 at 5:00 PM, Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. Bit of a jump to conclusions, don't you think? What makes you think I don't recognize the code I changed? @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
Re: Android JUnit Tests Now Pass
On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve agri...@chromium.org wrote: I agree that significant changes should be reviewed first. But for the most part Cordova is a review-after-commit kind of place, No, it's not. Cordova is only like that because you consistently make it like that. Constantly committing to master without any review at all makes it next to impossible for anyone else to work on the project. You can tell that this is the case, because you own the majority of the commits over the last few months. That's not normal for a codebase this size with this many contributors. This is why we have topic branches, and we've had this discussion with you numerous times about using them. This is also why I write e-mails trying to get buy-in to what I want to do instead of just landing features straight on master that could break everything. and this change didn't touch any code that we release (strictly tests... that have been broken for a very long time), so I don't think it qualifies. I'll admit that the tests were a bit of the wild west. That said, there was always an understanding that tests would be added to and improved upon and not removed. Marcel and I probably wouldn't have had half the e-mails that we have had if it wasn't us arguing over whether to delete tests. I know it's frustrating to have to wait on other people, since people are human, get sick, and have to take care of others when they're sick. That said, it's equally frustrating to come back from vacation, or wake up from a nap after driving someone from the hospital and see that critical code that was a major issue only six months ago got accidentally removed in a sweeping change, along with other use cases. That's what happened yesterday, and that's why I got frustrated. If I'm having a bad day already, a random refactor that just gets dropped without at least a head's up beforehand makes it worse. I've been on both sides of the issue with this. I remember getting extremely frustrated with Bryce when we designed CordovaWebView, especially since my design had less of a change to the code. I thought things were moving too slowly, but at the end of the day we did produce something that a lot of people seem to use (at least that's what the sample repo I have on GitHub tells me, the GitHub analytics tools are all I have to go on). That said, we didn't ship that until it was mostly ready, and other than an awkward presentation at PhoneGap Day, it was mostly fine. I'm glad I didn't just merge my crap in and just unilaterally introduce that feature, since back then we could still get away with that technically. But yeah, can we have things on feature branches if they're that big, and then wait maybe 24 hours before dropping something like that? I'm not talking like a simple JIRA fix, but something that large should have been a pull request or on its own branch or something. On Thu, Feb 12, 2015 at 4:07 AM, Jesse purplecabb...@gmail.com wrote: You may or may not, but I think it would be nice to let others review your (significant) changes before dumping them to master. On Feb 11, 2015, at 6:34 PM, Andrew Grieve agri...@chromium.org wrote: On Wed, Feb 11, 2015 at 5:00 PM, Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. Bit of a jump to conclusions, don't you think? What makes you think I don't recognize the code I changed? @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all.
Re: Android JUnit Tests Now Pass
Sounds like you've been having a rough time. :( Hope you get through it. Believe me when I say I hear you loud and clear about making changes on feature branches. I just don't think this one fits. - No one (other than me) has touched the tests since September of last year, so it's unlikely that a change would cause merge conflicts. - The state of the tests show that they are not viewed as that important (at least not important enough for anyone other than me to have been working on them) - Anything I do to them doesn't affect shipping code. No risk. I find it hard to believe that my making changes contributes in a significant way to having people avoid working on Android. Perhaps being overly abrasive via email JIRA would be a deterrent though... On Thu, Feb 12, 2015 at 11:10 AM, Joe Bowser bows...@gmail.com wrote: On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve agri...@chromium.org wrote: I agree that significant changes should be reviewed first. But for the most part Cordova is a review-after-commit kind of place, No, it's not. Cordova is only like that because you consistently make it like that. Constantly committing to master without any review at all makes it next to impossible for anyone else to work on the project. You can tell that this is the case, because you own the majority of the commits over the last few months. That's not normal for a codebase this size with this many contributors. This is why we have topic branches, and we've had this discussion with you numerous times about using them. This is also why I write e-mails trying to get buy-in to what I want to do instead of just landing features straight on master that could break everything. and this change didn't touch any code that we release (strictly tests... that have been broken for a very long time), so I don't think it qualifies. I'll admit that the tests were a bit of the wild west. That said, there was always an understanding that tests would be added to and improved upon and not removed. Marcel and I probably wouldn't have had half the e-mails that we have had if it wasn't us arguing over whether to delete tests. I know it's frustrating to have to wait on other people, since people are human, get sick, and have to take care of others when they're sick. That said, it's equally frustrating to come back from vacation, or wake up from a nap after driving someone from the hospital and see that critical code that was a major issue only six months ago got accidentally removed in a sweeping change, along with other use cases. That's what happened yesterday, and that's why I got frustrated. If I'm having a bad day already, a random refactor that just gets dropped without at least a head's up beforehand makes it worse. I've been on both sides of the issue with this. I remember getting extremely frustrated with Bryce when we designed CordovaWebView, especially since my design had less of a change to the code. I thought things were moving too slowly, but at the end of the day we did produce something that a lot of people seem to use (at least that's what the sample repo I have on GitHub tells me, the GitHub analytics tools are all I have to go on). That said, we didn't ship that until it was mostly ready, and other than an awkward presentation at PhoneGap Day, it was mostly fine. I'm glad I didn't just merge my crap in and just unilaterally introduce that feature, since back then we could still get away with that technically. But yeah, can we have things on feature branches if they're that big, and then wait maybe 24 hours before dropping something like that? I'm not talking like a simple JIRA fix, but something that large should have been a pull request or on its own branch or something. On Thu, Feb 12, 2015 at 4:07 AM, Jesse purplecabb...@gmail.com wrote: You may or may not, but I think it would be nice to let others review your (significant) changes before dumping them to master. On Feb 11, 2015, at 6:34 PM, Andrew Grieve agri...@chromium.org wrote: On Wed, Feb 11, 2015 at 5:00 PM, Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. Bit of a jump to conclusions, don't you think? What makes you think I don't recognize the code I changed? @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just
Re: Android JUnit Tests Now Pass
This commit may not have warranted this discussion. I think we agree that large changes/commits should be on feature branches, and discussed before being merged. Let's go with that. On Feb 12, 2015, at 8:49 AM, Andrew Grieve agri...@chromium.org wrote: Sounds like you've been having a rough time. :( Hope you get through it. Believe me when I say I hear you loud and clear about making changes on feature branches. I just don't think this one fits. - No one (other than me) has touched the tests since September of last year, so it's unlikely that a change would cause merge conflicts. - The state of the tests show that they are not viewed as that important (at least not important enough for anyone other than me to have been working on them) - Anything I do to them doesn't affect shipping code. No risk. I find it hard to believe that my making changes contributes in a significant way to having people avoid working on Android. Perhaps being overly abrasive via email JIRA would be a deterrent though... On Thu, Feb 12, 2015 at 11:10 AM, Joe Bowser bows...@gmail.com wrote: On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve agri...@chromium.org wrote: I agree that significant changes should be reviewed first. But for the most part Cordova is a review-after-commit kind of place, No, it's not. Cordova is only like that because you consistently make it like that. Constantly committing to master without any review at all makes it next to impossible for anyone else to work on the project. You can tell that this is the case, because you own the majority of the commits over the last few months. That's not normal for a codebase this size with this many contributors. This is why we have topic branches, and we've had this discussion with you numerous times about using them. This is also why I write e-mails trying to get buy-in to what I want to do instead of just landing features straight on master that could break everything. and this change didn't touch any code that we release (strictly tests... that have been broken for a very long time), so I don't think it qualifies. I'll admit that the tests were a bit of the wild west. That said, there was always an understanding that tests would be added to and improved upon and not removed. Marcel and I probably wouldn't have had half the e-mails that we have had if it wasn't us arguing over whether to delete tests. I know it's frustrating to have to wait on other people, since people are human, get sick, and have to take care of others when they're sick. That said, it's equally frustrating to come back from vacation, or wake up from a nap after driving someone from the hospital and see that critical code that was a major issue only six months ago got accidentally removed in a sweeping change, along with other use cases. That's what happened yesterday, and that's why I got frustrated. If I'm having a bad day already, a random refactor that just gets dropped without at least a head's up beforehand makes it worse. I've been on both sides of the issue with this. I remember getting extremely frustrated with Bryce when we designed CordovaWebView, especially since my design had less of a change to the code. I thought things were moving too slowly, but at the end of the day we did produce something that a lot of people seem to use (at least that's what the sample repo I have on GitHub tells me, the GitHub analytics tools are all I have to go on). That said, we didn't ship that until it was mostly ready, and other than an awkward presentation at PhoneGap Day, it was mostly fine. I'm glad I didn't just merge my crap in and just unilaterally introduce that feature, since back then we could still get away with that technically. But yeah, can we have things on feature branches if they're that big, and then wait maybe 24 hours before dropping something like that? I'm not talking like a simple JIRA fix, but something that large should have been a pull request or on its own branch or something. On Thu, Feb 12, 2015 at 4:07 AM, Jesse purplecabb...@gmail.com wrote: You may or may not, but I think it would be nice to let others review your (significant) changes before dumping them to master. On Feb 11, 2015, at 6:34 PM, Andrew Grieve agri...@chromium.org wrote: On Wed, Feb 11, 2015 at 5:00 PM, Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. Bit of a jump to conclusions, don't you think? What makes you think I don't recognize the code I changed? @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I
Re: Android JUnit Tests Now Pass
I see no situation where we don't want a feature branch vetted by 1 person before we land anything on master …short of fixing broken tests. I assume good faith. Joe: you had a bad day and, I think, you still feel mistrust after previous commits landing on master stalling out your work last summer. Lets put that behind us. Andrew pls fire a ping to the list w/ a PR for anything aiming to live on Android master until earn Joe's trust back. And lets avoid the editorial about motivations. We all want the same thing here: work on a kick ass open source project that makes a difference. On Thu, Feb 12, 2015 at 9:31 AM, Jesse purplecabb...@gmail.com wrote: This commit may not have warranted this discussion. I think we agree that large changes/commits should be on feature branches, and discussed before being merged. Let's go with that. On Feb 12, 2015, at 8:49 AM, Andrew Grieve agri...@chromium.org wrote: Sounds like you've been having a rough time. :( Hope you get through it. Believe me when I say I hear you loud and clear about making changes on feature branches. I just don't think this one fits. - No one (other than me) has touched the tests since September of last year, so it's unlikely that a change would cause merge conflicts. - The state of the tests show that they are not viewed as that important (at least not important enough for anyone other than me to have been working on them) - Anything I do to them doesn't affect shipping code. No risk. I find it hard to believe that my making changes contributes in a significant way to having people avoid working on Android. Perhaps being overly abrasive via email JIRA would be a deterrent though... On Thu, Feb 12, 2015 at 11:10 AM, Joe Bowser bows...@gmail.com wrote: On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve agri...@chromium.org wrote: I agree that significant changes should be reviewed first. But for the most part Cordova is a review-after-commit kind of place, No, it's not. Cordova is only like that because you consistently make it like that. Constantly committing to master without any review at all makes it next to impossible for anyone else to work on the project. You can tell that this is the case, because you own the majority of the commits over the last few months. That's not normal for a codebase this size with this many contributors. This is why we have topic branches, and we've had this discussion with you numerous times about using them. This is also why I write e-mails trying to get buy-in to what I want to do instead of just landing features straight on master that could break everything. and this change didn't touch any code that we release (strictly tests... that have been broken for a very long time), so I don't think it qualifies. I'll admit that the tests were a bit of the wild west. That said, there was always an understanding that tests would be added to and improved upon and not removed. Marcel and I probably wouldn't have had half the e-mails that we have had if it wasn't us arguing over whether to delete tests. I know it's frustrating to have to wait on other people, since people are human, get sick, and have to take care of others when they're sick. That said, it's equally frustrating to come back from vacation, or wake up from a nap after driving someone from the hospital and see that critical code that was a major issue only six months ago got accidentally removed in a sweeping change, along with other use cases. That's what happened yesterday, and that's why I got frustrated. If I'm having a bad day already, a random refactor that just gets dropped without at least a head's up beforehand makes it worse. I've been on both sides of the issue with this. I remember getting extremely frustrated with Bryce when we designed CordovaWebView, especially since my design had less of a change to the code. I thought things were moving too slowly, but at the end of the day we did produce something that a lot of people seem to use (at least that's what the sample repo I have on GitHub tells me, the GitHub analytics tools are all I have to go on). That said, we didn't ship that until it was mostly ready, and other than an awkward presentation at PhoneGap Day, it was mostly fine. I'm glad I didn't just merge my crap in and just unilaterally introduce that feature, since back then we could still get away with that technically. But yeah, can we have things on feature branches if they're that big, and then wait maybe 24 hours before dropping something like that? I'm not talking like a simple JIRA fix, but something that large should have been a pull request or on its own branch or something. On Thu, Feb 12, 2015 at 4:07 AM, Jesse purplecabb...@gmail.com wrote: You may or may not, but I think it would be nice to let others review your (significant)
Re: Android JUnit Tests Now Pass
Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
RE: Android JUnit Tests Now Pass
This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. (https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
Re: Android JUnit Tests Now Pass
I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
Re: Android JUnit Tests Now Pass
+1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
Re: Android JUnit Tests Now Pass
I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
Re: Android JUnit Tests Now Pass
I'm reverting this now, 37 tests with 4 failures is much better than roughy 20 tests with 0 failures. (I didn't run the refactored tests, since there's no point if critical tests are missing). On Wed Feb 11 2015 at 2:01:51 PM Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
Re: Android JUnit Tests Now Pass
Sorry for the scare. Maybe I wasn't clear, but I did not delete any failing tests. I *fixed* the failing tests by having them wait on mutexes instead of using Thread.sleep(). What I *did* delete: - Duplicate tests - Massive amounts of copy paste (refactor from having one activity per-test, to having all tests share an activity and passing the URL to load via the Intent) - Tests that had no assertions Examples of deletes: LifecycleTest: - https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-7753f0a08baabc682815c620354bb33c - It has no assertions (nothing was being tested) PluginManagerTest: - https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-5fa78a4d2b5c99cd3d1b923edfd07ad6 - It has only one test, and it's commented out. CordovaTest: - https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-0b78d46f0e845439f01d054238a17afb - has only commented out tests GapClientTest: - https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-d71ae4e49caa340447954c24f9670eba - Asserts the classnames of things. This is already covered by other tests FixWebView: - https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-66f4f1b9fd4ceab56d347e88d574518b - Unused class IntentUriOverrideTest - This one shouldn't have been deleted. Got me here! It's easy to put back though (has only one assertion) I spent all of this time because *I agree* there is a lot of value in tests. I spent a lot of time to ensure that test app still runs in stand-alone mode, and that no real test was lost in this change. Please take some time to look at this before attacking it. I put a great deal of care into it. I'll wait before reverting the revert, but I don't see where the anger is coming from. Didn't we just recently discuss the desire to clean up the tests? On Wed, Feb 11, 2015 at 5:28 PM, Joe Bowser bows...@gmail.com wrote: I'm reverting this now, 37 tests with 4 failures is much better than roughy 20 tests with 0 failures. (I didn't run the refactored tests, since there's no point if critical tests are missing). On Wed Feb 11 2015 at 2:01:51 PM Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :)
Re: Android JUnit Tests Now Pass
On Wed, Feb 11, 2015 at 5:00 PM, Jesse purplecabb...@gmail.com wrote: +1 Revert And please let's stop deleting what other people wrote just because we don't recognize it. These things should require discussion. Bit of a jump to conclusions, don't you think? What makes you think I don't recognize the code I changed? @purplecabbage risingj.com On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser bows...@gmail.com wrote: I think we should revert this refactor. With the new refactored tests, they may pass but we lost a lot of the useful tests that we once had and these new tests have no value. I don't know why you took it upon yourself to throw away all the JUnit tests that didn't pass, but that misses the point. I would have rather had the old tests expanded upon instead of just deleted on your personal whim. I honestly don't know what to say, I know that we have a terrible working relationship at best, but this actually is making the project worse intentionally for unknown reasons. In fact, I would almost say that this is purely a malicious change driven by ego, since I can't see a technical reason for any of it. On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser bows...@gmail.com wrote: I think there's a lot of value in the Unit Tests, having wrote the majority of them initially. If I wasn't dealing with everyone in my house getting sick, I'd check to make sure these tests were still testing what I intended them to test, since we have a habit of losing the intent behind the test every time we do a refactor. Of course, if we're going to throw away the embedded WebView case, then maybe there's not value after all. On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve agri...@chromium.org wrote: Does travis provide Android emulators? I'd guess it'd be too slow to put on Travis. And honestly, there's still not a lot of value in the unit tests atm. On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc mura...@microsoft.com wrote: This is great news! I've finally got the android travis enabled too. We have jshint and jasmine test coverage on every commit now. ( https://travis-ci.org/apache/cordova-android/builds/50295748) Now that we're passing all junit tests, I think the next step for us should be to integrate junit tests with travis. What do you think? -Original Message- From: agri...@google.com [mailto:agri...@google.com] On Behalf Of Andrew Grieve Sent: Tuesday, February 10, 2015 7:14 PM To: dev Subject: Android JUnit Tests Now Pass Spent some time cleaning up the tests. Certainly they could be made even better made to test more things, but at least they pass now :) Much of the change was deleting copy paste, and deleting commented out tests: 53 files changed, 941 insertions(+), 2610 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
