Enabling default app/page templates for user workspace

[hutchgrant]

Related Issue

Resolves #30 #32

Summary of Changes

• fixed init.js context for better readability, added appTemplatePath and pageTemplatePath var
• removed from init.js context pageTemplate and appTemplate
• updated scaffold to use full appTemplatePath for routes import
• updated scaffold for pageComponent creation from page-template. If md file uses 'page' for its template(which it will use by default), it will use the context's page-template path or if not 'page' template, will use user workspace's templates directory
• updated tests to double check not just nested paths but also public directory, hello world and index.html
• added tests for mock-app without page and app templates

[thescientist13]

Hmm, not fond about the introduction of testUserWorkspace.

In general, I favor tests that are very procedural and decoupled and don't share very much logic (business, convenience, or otherwise). Tests aren't necessarily the best case to go too DRY in my experience. I know there was a whenSerialized before and I had removed that intentionally in #51 for the same reason.

Maybe the real goal here should be to create a directory of specs for the CLI (tests/cli/) and take a queue from webpack and organize our specs according to cases? So it looks like we might have three here?

Either way, I would prefer to walk back the testUserWorkspace function at minimum and try and keep each describe as encapsulated as possible with the code it needs. I think TestSetup is still useful though, and can still be used across all cases since it's pretty simple and narrow in scope.

[thescientist13]

Also, is #30 actually resolved here, it wasn't clear. Seems unrelated to #32 though, but I could be wrong. Hard to tell how each particular feature is getting implemented if so. 😞 🙈

[hutchgrant]

There is a solid reason for it. We're writing duplicate tests for different cases.

We need to test nested, hello, public directory, and js bundles in all scenarios. So I have to disagree with you on that.

#53 (comment)

[hutchgrant]

Most of #30 is resolved already in nested directory PR. I simply set it so the default 'page' template would be set to the templates/page-template.js that we include.

The label var of #30 is not approachable from this project. We'd have to rewrite the wc-md-loader and somehow incorporate it into this project. We need the name of the label both in the loader and in the scaffold.js. The only way to do that properly would be to have the webpack plugin handle the adding the new wc-md-element to the page-template. This would require some further research.

[thescientist13]

So, I think we can both agree the tests could probably be better organized and opportunities to improve here, so let's start from there and continue the discussion in this issue.

However, in a PR already combining two features, I'm not keen on this refactoring on top of it all unfortunately and I don't necessarily concur that making our tests more DRY or convenient for us is should necessarily be the prioritizing factor in terms of how we approach our tests, is all.

[hutchgrant]

Then let's do a separate RFC PR for the tests then considering we have to merge tests from all these PRs first.

3/4 outstanding PRs contain additional tests. If you plan on moving them or reorganizing them, that's a big change and should be dealt with in a different PR. The issues this PR solves are what's important. It has tests for those changes. Changes to these tests(and all the other tests combined) need to be done separately as that has nothing to do with this PR.

[thescientist13]

The issues this PR solves are what's important. It has tests for those changes. Changes to these tests(and all the other tests combined) need to be done separately as that has nothing to do with this PR.

That's not really playing fair, tbh. 😢

The issues this PR solves are important of course, and for that reason should be the only thing being talked about in this PR, but, it isn't, and that's because refactoring unit tests is not the right place / time for something like that.

I hope you know you can always ask first, as I think by now the project has well established a consistent 1 issue : 1 PR workflow, so if having done #11 the way we did didn't solidify that further, then aside from putting language like that in CONTRIBUTING.md, not sure what else I should do to help us avoid these scenarios? 😕

[hutchgrant]

So you want me to put it back then and then add the duplicate tests? because that's the only way to do it.

[thescientist13]

Oh, with this PR we should be able to remove this TODO line in the README, right?
https://github.com/ProjectEvergreen/greenwood#project-structure

[thescientist13]

😎