honor app name in arguments

[jannyHou]

Description

Fixing the test failure in strongloop/loopback-cli#71, it's also a missing feature.

Related issues

strongloop/loopback-cli#76

• New tests added or existing tests modified to cover all changes
• Code conforms with the style
  guide

[b-admike]

Do we have a test for when validation fails for the required name? Otherwise, LGTM.

[b-admike]

nitpick: can we have the curly brackets for the if condition above this else statement?

[bajtos]

No need to use else after return.

if (valid === true) return;
helpers.reportValidationError(valid, this.log);
this.name = undefined;

[bajtos]

I find this slightly confusing - how is the base property related to the model name provided via CLI arguments? Should you perhaps check that name property was set to Product instead?

[jannyHou]

@bajtos if you look at the tests above, all the model cli tests create a model called Product, and check the result by asserting the model config is re-generated.

The test before it sets the base to CustomModel, therefore my test reset it to PersistedModel and assert accordingly.

Should you perhaps check that name property was set to Product instead?

I could add expect(product).to.have.property('name', 'Product');, but even the test fail this assertion would still be true :)

[bajtos]

The test before it sets the base to CustomModel, therefore my test reset it to PersistedModel and assert accordingly.

Could you please add a comment to explain this?

In general, tests should be independent on each other and don't rely on side effects.

but even the test fail this assertion would still be true :)

Isn't the same true for your current test in case only this single test is executed and all other tests are skipped? Either via it.only or in CLI via mocha -g "loopback:model generator.*honors the model name in arg".

I think you should explicitly set a different base model in this test, one that's not the default and also not used by any other tests around. Perhaps Model may work? Then verify that this explicit value was written to the file.

[jannyHou]

@bajtos Hmm I see your concern. I rewrote the test, hopefully it makes more sense.

[jannyHou]

@b-admike Sorry i missed your comment, we usually don't test the printed warning. And since the name provided in withPrompts will always override the valid/invalid arguments, I couldn't think of a way to test this scenario.

I manually verified it:

jannyhous-mbp:testingapp jannyhou$ slc loopback:model %$@%#%%
Name cannot contain special characters /[\/@\s\+%:\.]/ %%#%%
? Enter the model name:

The invalid name warning is printed and the default name is reset to undefined. But good catch though :) turns out the helper function doesn't really print it, I switch to this.log() instead.

[bajtos]

Much better 👏

Please clean up the git commit history before landing.