feat(android-studio): display app name as project name

[sagrawal31]

Platforms affected

• Android

Motivation and Context

Closes #1172

Testing

Tried locally but couldn't run (probably I have to read it more carefully).

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary

[breautek]

Tried locally but couldn't run (probably I have to read it more carefully).

For testing, I'd advise running npm test. This will make sure nothing existing broke by the introduction of your change, as well run the linter to check if the coding style is acceptable. PR checks will do more extensive testing in multiple environments.

You can also test locally by adding your custom platform by doing cordova platform add file:/path/to/your-cordova-android-fork which will install the current version/state of your cordova-android directory.

[sagrawal31]

Oh! thanks again @breautek I was looking for that only to create a platform locally.

[sagrawal31]

Tried with cordova platform add file:/path/to/your-cordova-android-fork.. worked fine 😎 Now running the test.

Pardon my failing test cases, this is my first-time contribution to cordova-android.

[sagrawal31]

Ran npm test, not sure how to take care of this in test files (digging deeper)-

[breautek]

The tests uses fake paths cause they don't actually create files, they do this by mocking the real implementations of the filesystem.

Looks like the tests will have to be updated. I'd probably would spy on fs.ensureDirSync and fs.writeFileSync to prevent actual file IO. (in most of those tests we don't actually care if writeNameForAndroidStudio because testing that is out of scope)

Then ideally we will have some specific tests that will test the new writeNameForAndroidStudio function itself by ensuring those two filesystem api calls are called with the proper parameters. We don't need to test if the implementation of those two FS calls works because we can assume NodeJS / fs-extra plugin authors have done their own testing, so in theory as long as we call them with the proper parameters, then everything should be okay 😁

[sagrawal31]

We don't need to test if the implementation of those two FS calls works because we can assume NodeJS / fs-extra plugin authors have done their own testing

Make sense. Most of the time (white writing JUnit test cases), my mind keep asking questions and then I used to reach to the same conclusion that the author of XYZ must have done their own testing 😄

Well, adding spyOn(create, 'writeNameForAndroidStudio'); started to pass the tests. But it's showing some red mark in the last result. Not sure if those are okay-

[breautek]

Well, adding spyOn(create, 'writeNameForAndroidStudio'); started to pass the tests. But it's showing some red mark in the last result. Not sure if those are okay-

In jasmine, by default when you spy on a function, it will not call on the original implementation which is why it seemed to fix everything. I think this is okay since we don't need to test writeNameForAndroidStudio in every single test case.

But we should have something like the following somewhere:

describe('writeNameForAndroidStudio', () => {
   it('should call ensureDirSync with path', () => {
    spyOn(fs, 'ensureDirSync');
    blah.writeNameForAndroidStudio(path); // Whatever what the interface is... 
    expect(fs.ensureDirSync).toHaveBeenCalledWith("thePathValue that is expected to have been passed into ensureDirSync");
  });

  it('should call wrilteFileSync', () => {
    // basically the same for above but with writeFileSync instead.
  });
});

That way the writeNameForAndriodStudio is also tested.

[sagrawal31]

Thanks, @breautek for the guidance. That hint was more than sufficient to write the test cases :)

[breautek]

This is really good, also like that you added the comment explaining why we are adding that file, and the PR checks are green which is a good sign!

I just have a few nitpicks, namely the use of var keyword and I'd prefer setting up the spies in a beforeEach block since I think we'd want those two functions to be always spied on, even in potential future tests that may be added later.

Once these changes are committed, ping me and I'll give the PR a manual test.

[sagrawal31]

This is really good, also like that you added the comment explaining why we are adding that file

Thanks for the appreciation!

and the PR checks are green which is a good sign!

Kudos to you for making it green.

I just have a few nitpicks, namely the use of var keyword and I'd prefer setting up the spies in a beforeEach block since I think we'd want those two functions to be always spied on, even in potential future tests that may be added later.

Makes a lot of sense. I ❤️ doing nitpicks. Thanks for pointing this out.

Once these changes are committed, ping me and I'll give the PR a manual test.

It's done. By the time you will read this comment, yellow dots will be turned green and then you can test locally.

Thanks again for your time.

[erisu]

Code LGTM

[breautek]

FYI: I edited the main post to include the GitHub Keyword so that the main issue ticket will be closed when we merge this in.

[breautek]

Ignore my message about things aren't working 😅 I retried the steps and it started working, I probably just accidentally used the wrong cordova-android fork the first time 😅

[sagrawal31]

Ignore my message about things aren't working 😅 I retried the steps and it started working, I probably just accidentally used the wrong cordova-android fork the first time 😅

Thanks, no problem 😄 I was about to comment whether you have used the correct branch or not 😃

[breautek]

Thank you @sagrawal31 for your time and effort into preparing this PR. I hope we see more in the future. If you're not already, I invite you to our Slack community channel and our Dev Mailing List where you can stay up to date or contribute to development decisions.

[sagrawal31]

Thanks for your mentoring & guidance @breautek. Happy to contribute more.