Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve GH-539 & GH-540 on master #542

Merged
merged 6 commits into from
Nov 12, 2018
Merged

Resolve GH-539 & GH-540 on master #542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
898a6a8
Add a unit test to test source-file target-dir /app/src/main/…
kkirby Nov 8, 2018
fb1dfb2
unit test uninstall of <source-file> with app dest
Nov 11, 2018
c2f6631
GH-539 fix destination path fallback
Nov 11, 2018
3caefca
unit test source-file with custom lib target-dir
Nov 9, 2018
c15312e
unit test uninstall of <source-file> with app dest
Nov 11, 2018
ef493b4
GH-540 fix for source-file with app target-dir
Nov 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions bin/templates/cordova/lib/pluginHandlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var handlers = {
if (!obj.src) throw new CordovaError(generateAttributeError('src', 'source-file', plugin.id));
if (!obj.targetDir) throw new CordovaError(generateAttributeError('target-dir', 'source-file', plugin.id));

var dest = studioPathRemap(obj);
var dest = getInstallDestination(obj);

if (options && options.force) {
copyFile(plugin.dir, obj.src, project.projectDir, dest, !!(options && options.link));
Expand All @@ -35,7 +35,7 @@ var handlers = {
}
},
uninstall: function (obj, plugin, project, options) {
var dest = studioPathRemap(obj);
var dest = getInstallDestination(obj);

// TODO: Add Koltin extension to uninstall, since they are handled like Java files
if (obj.src.endsWith('java')) {
Expand Down Expand Up @@ -292,9 +292,15 @@ function generateAttributeError (attribute, element, id) {
return 'Required attribute "' + attribute + '" not specified in <' + element + '> element from plugin: ' + id;
}

function getInstallDestination (obj) {
return studioPathRemap(obj) ||
path.join(obj.targetDir, path.basename(obj.src));
}

function studioPathRemap (obj) {
// If a Java file is using the new directory structure, don't penalize it
if (!obj.targetDir.includes('app/src/main')) {
// If any source file is using the app new directory structure,
// don't penalize it
if (!obj.targetDir.includes('app')) {
if (obj.src.endsWith('.java')) {
return path.join('app/src/main/java', obj.targetDir.substring(4), path.basename(obj.src));
} else {
Expand Down
6 changes: 6 additions & 0 deletions spec/fixtures/org.test.plugins.dummyplugin/plugin.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@

<source-file src="src/android/DummyPlugin.java"
target-dir="src/com/phonegap/plugins/dummyplugin" />
<source-file src="src/android/DummyPlugin2.java"
target-dir="app/src/main/src/com/phonegap/plugins/dummyplugin" />
<source-file src="src/android/TestLib.jar"
target-dir="app/libs" />
<source-file src="src/android/TestAar.aar"
target-dir="app/libs" />
<lib-file src="src/android/TestLib.jar" />
</platform>
</plugin>
1 change: 1 addition & 0 deletions spec/fixtures/org.test.plugins.dummyplugin/src/android/DummyPlugin2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
./org.test.plugins.dummyplugin/src/android/DummyPlugin2.java
1 change: 1 addition & 0 deletions spec/fixtures/org.test.plugins.dummyplugin/src/android/TestAar.aar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
./org.test.plugins.dummyplugin/src/android/TestAar.aar
47 changes: 47 additions & 0 deletions spec/unit/pluginHandlers/handlers.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,25 @@ describe('android project handler', function () {
android['source-file'].install(valid_source[0], dummyPluginInfo, dummyProject);
}).toThrow(new Error('"' + target + '" already exists!'));
});

it('Test#007 : should allow installing sources using proper path', function () {
android['source-file'].install(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
expect(copyFileSpy)
.toHaveBeenCalledWith(dummyplugin, 'src/android/DummyPlugin2.java', temp, path.join('app/src/main/src/com/phonegap/plugins/dummyplugin/DummyPlugin2.java'), false);
});

// TODO: renumber these tests and other tests below
it('Test#007a : should allow installing lib file from sources using proper path', function () {
android['source-file'].install(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
expect(copyFileSpy)
.toHaveBeenCalledWith(dummyplugin, 'src/android/TestLib.jar', temp, path.join('app/libs/TestLib.jar'), false);
});

it('Test#007b : should allow installing aar file from sources using proper path', function () {
android['source-file'].install(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
expect(copyFileSpy)
.toHaveBeenCalledWith(dummyplugin, 'src/android/TestAar.aar', temp, path.join('app/libs/TestAar.aar'), false);
});
});

describe('of <framework> elements', function () {
Expand Down Expand Up @@ -251,6 +270,10 @@ describe('android project handler', function () {
});
});

// TODO:
// - merge tests of <source-file> elements into single describe block
// (with proper beforeEach/afterEach)
// - renumber the tests after Test#019
describe('of <source-file> elements', function () {
it('Test#019 : should remove stuff by calling common.deleteJava for Android Studio projects', function () {
android['source-file'].install(valid_source[0], dummyPluginInfo, dummyProject);
Expand All @@ -259,6 +282,30 @@ describe('android project handler', function () {
});
});

describe('of <source-file> element, with specific app target-dir', function () {
it('Test#019a : should remove stuff by calling common.deleteJava for Android Studio projects, with specific app target-dir', function () {
android['source-file'].install(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
android['source-file'].uninstall(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
expect(deleteJavaSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/src/com/phonegap/plugins/dummyplugin/DummyPlugin2.java'));
});
});

describe('of <source-file> element, with JAR in specific app target-dir', function () {
it('Test#019b : should remove stuff by calling common.deleteJava for Android Studio projects, with JAR into specific app target-dir', function () {
android['source-file'].install(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
android['source-file'].uninstall(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/TestLib.jar'));
});
});

describe('of <source-file> element, with AAR in specific app target-dir', function () {
it('Test#019c : should remove stuff by calling common.deleteJava for Android Studio projects, with JAR into specific app target-dir', function () {
android['source-file'].install(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
android['source-file'].uninstall(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/TestAar.aar'));
});
});

describe('of <framework> elements', function () {

var someString = jasmine.any(String);
Expand Down