CB-11964: Do a clean when installing a plugin to get around the bug #339
Conversation
|
OK, so, WTF was that? It appears that we have a test suite problem. |
…ment, code duplication is needed because of builderEnv
Current coverage is 35.66% (diff: 71.42%)@@ master #339 diff @@
==========================================
Files 12 12
Lines 1007 1015 +8
Methods 198 200 +2
Messages 0 0
Branches 168 168
==========================================
+ Hits 354 362 +8
Misses 653 653
Partials 0 0
|
| //the Gradle bug introduced by the Android Gradle Plugin Version 2.2 | ||
| //TODO: Delete when the next version of Android Gradle plugin comes out | ||
|
|
||
| this.clean(); |
There was a problem hiding this comment.
Is it out of promise chain intentionally? I think that this.clean() returns a promise.
I've tried to merge it to the promise chain, but got the following exception upon cordova platform add:
Error: ENOENT: no such file or directory, open 'C:\Cordova\foo\platforms\android\assets\www\cordova_plugins.js'
at Error (native)
at Object.fs.openSync (fs.js:549:18)
at Object.fs.writeFileSync (fs.js:1156:15)
at C:\Cordova\foo\platforms\android\cordova\node_modules\cordova-common\src\PluginManager.js:137:12
at _fulfilled (C:\Cordova\foo\platforms\android\cordova\node_modules\q\q.js:834:54)
at self.promiseDispatch.done (C:\Cordova\foo\platforms\android\cordova\node_modules\q\q.js:863:30)
at Promise.promise.promiseDispatch (C:\Cordova\foo\platforms\android\cordova\node_modules\q\q.js:796:13)
at C:\Cordova\foo\platforms\android\cordova\node_modules\q\q.js:857:14
at runSingle (C:\Cordova\foo\platforms\android\cordova\node_modules\q\q.js:137:13)
at flush (C:\Cordova\foo\platforms\android\cordova\node_modules\q\q.js:125:13)
I wonder why it works the way you put it and if it is dangerous to leave it like this.
There was a problem hiding this comment.
I agree it's dangerous, but I can't even get it to clean inside the promise chain. If we can get it to work in a safer way, that'd be better.
| @@ -349,13 +354,24 @@ Api.prototype.run = function(runOptions) { | |||
| */ | |||
| Api.prototype.clean = function(cleanOptions) { | |||
| var self = this; | |||
| try { | |||
There was a problem hiding this comment.
I think instead of doing this try catch in product code we could just mock Api.prototype.clean() in tests. Please see my attempt at doing so:
https://github.com/alsorokin/cordova-android/commit/3eae22f25887c3b84a20dfaf4bdca9e45d6e77e9
There was a problem hiding this comment.
That is a better solution.
|
Moving the example into core, tested it locally, everything appears to check out on MacOS. Hopefully there's no weird Windows issues. |
|
OK, there's something wrong with how we're cleaning in the chain. I think we need to do it later in the promise chain. I hate promise chains much more than what people called "callback hell". |
… errors that were happening when building
infil00p
changed the title
|
@alsorokin I'm going to keep the commits exploded and as is on this PR. If this looks good to you, I'll merge this in tonight. |
|
Tested it on Windows, it works! LGTM |
This is a quick fix for the problem, and it might fix some other build errors. I'm not sure about doing it here, because it might break more stuff, but hey, that's what PRs and CI are for. :)