Skip to content

CB-12361 : added unit-tests for getPlatformDetailsFromDir #578

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.

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

Merged
merged 2 commits into from
Jul 28, 2017

Conversation

audreyso
Copy link
Contributor

Platforms affected

What does this PR do?

added unit-tests for getPlatformDetailsFromDir

What testing has been done on this change?

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

Copy link
Contributor

@stevengill stevengill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for one comment. 100% npm run cover too!

it('should remove the cordova- prefix from the platform name for known platforms', function (done) {
platform_getPlatformDetails.platformFromName('cordova-ios');
expect(events.emit).toHaveBeenCalledWith('verbose', jasmine.stringMatching(/Removing "cordova-" prefix/));
expect(platform_getPlatformDetails.platformFromName('cordova-ios')).toBe('ios');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can take the expect from line 76 and combine it with line 74 and delete 76. So line 74 gets replaced by line 76. Line 75 should still pass

Copy link
Contributor Author

@audreyso audreyso Jul 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh okay do you mean just like this? @stevengill

 it('should remove the cordova- prefix from the platform name for known platforms', function (done) {
        expect(platform_getPlatformDetails.platformFromName('cordova-ios')).toBe('ios');
        expect(events.emit).toHaveBeenCalledWith('verbose', jasmine.stringMatching(/Removing "cordova-" prefix/));
        done();
 });

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup

@asfgit asfgit merged commit 02e734c into apache:master Jul 28, 2017
dpogue pushed a commit that referenced this pull request Oct 23, 2024
…n removePluginFromPlatform() (#913)

* plugin/remove.js: don't stomp opts.cli_variables in removePluginFromPlatform()

* undo change to line spacing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants