Location of files is needlessly opinionated

[electrode-io]

Location of files is needlessly opinionated

[JakobJingleheimer]

I was going to put together a PR (I did for a couple other things), but I anticipated a discussion because this an ideological change, and the current implementation is clearly a preference. If the Electrode team consents, I can make the PR.

I have 2 main reasons:

1. It's modular. This makes it easier and more logical to add/remove/update components. This is the intention of ES6 modules (whence import/export came).
2. This isn't the type of opinion that leads to convention over configuration. If Electrode was providing some shortcuts as a result of it (beyond what the underlying frameworks do), then sure. But it isn't. The convention would be some kind of path match that saves configuration, like tests/client/components/<ComponentName>.js automatically importing client/components/<ComponentName>/index.js, but that doesn't happen. All it does is enforce the Electrode team's preference for directory structure. On the flip-side, with them co-located:

// SomeComponent.spec.js
import Component from './';

But with my suggestion, you could do either—whichever fits that application and team.

I'm all for a good convention that saves configuration.

RE Already having the tools to change it: I'm saying with this simple update, overriding is not necessary. These sorts of things should not need to be overridden wholesale, especially because it's a pain when the underlying code is updated and the override is only providing a tiny tweak.

[lnaia]

IMHO:
Opinionated is good. It forces structure, coherence among projects using the same framework, and say, in the long run it reduces drag, specially when new team members come along. Big plus if the team member has already worked with electrode in the past.

A good, very good thing about electrode is that the entire building layer of the project is abstracted from the project itself under the archetypes. You can always write your own archetype, inherit the current one, and change only what you need, or completely rewrite it, for your specific edge case.
I'm having high hopes, when the project goes from webpack1 to webpack2, and I don't need to change almost anything on the current project.

There are a ton of non opinionated boilerplates out there, although the enforcement of these rules, can pave way for interesting things like convention over configuration, (Rails comes to mind), and diminish the fragmentation of effort of the project.

tl; dr;
You already have the tools to change what you need, using your own archetype, more info: http://www.electrode.io/docs/what_are_archetypes.html
edit: or just make a PR 👍

[JakobJingleheimer]

I see a 👍 from the only dissenter (@darkbls), so I'll put together a PR for this.

[jchip]

is the karma test files the only ones you want to be able to structure differently?

[JakobJingleheimer]

I think so? It looks like that's where unit tests are run, no? (We have a separate platform for integration tests)

[jchip]

The separation of test code from source in different directory is so we can do mass processing of the client code with wildcats without worrying about the test code getting in the way, or constantly having to worry about excluding them. It also allow us to run all the tests by simply pointing to test/client/spec, without having to glob for *.spec.js*. This structure is the most common as I can see in all the modules I've looked at. Generally src or lib with test at the same level.

If naming the directory test or leaving tests under spec is not your preference, I might consider some option, but colocating tests along with the source is quite different from our overall setup, and not something I've ever encountered.

[JakobJingleheimer]

Yes, I understood why you were doing it—it was how I organised things when I first started out.

Collocating them in quite common. If you'd like, I can provide some references to major projects that do it that way.

[JakobJingleheimer]

Actually, I'll just throw this one out there: AngularJS Styleguide

This was basically "blessed" by the Angular team but not officially dictated because they didn't want to prescribe such minutiae. It was pretty universally adopted anyway though.

This is also essential for feature toggling and a distrubuted team.

[jchip]

Thanks for the link.

The guide calls for "server integration or test multiple components in a separate tests folder", but then also "Place unit test files (specs) side-by-side with your client code". That's kind of conflicting and inconsistent IMO.

So the first 5 whys sound like they are just for convenient's sake and force user to remember the tests. I don't quite get the last why though.

Opinion wise, to me, I think keeping all specs in a separate test folder is consistent. More importantly, a lot of times there are mocks and fixtures, and I think they should be kept together with the tests.

Technical wise, in addition to the reasons I already given about not having to worry about excluding tests in our code, we also have linting rules that are different for tests. Having both test and source side by side really complicates that.

I am open to making things configurable, but I have to draw a line somewhere. While my pref is tests separate, I don't feel strongly opinion wise about it, but it does complicates things for our code and setup so I prefer not to do this.

Thanks.

[JakobJingleheimer]

TL;DR I think having a configuration for this would not work because of all the moving pieces you mentioned: applying different linting rules based on N-possibilities AND globbing 😱

---

The guide calls for "server integration or test multiple components in a separate tests folder", but then also "Place unit test files (specs) side-by-side with your client code". That's kind of conflicting and inconsistent IMO.

Why? A unit test applies to 1 file; an integration test applies to many—so by nature you can't co-locate integration tests (because it doesn't exist in multiple places at once).

It's fairly common to do integration tests for views/pages (which consume components):

│ client
│ ├─┬ components
│   ├─┬ banner
│     ├── index.css
│     ├── index.jsx
│     ├── index.mock.jsx // as needed
│     └── index.spec.jsx
│   ├─┬ form
│     ├── index.css
│     ├── index.jsx // uses ../input
│     └── index.spec.jsx
│   └─┬ input
│     ├── index.css
│     ├── index.jsx
│     ├── index.mock.jsx // as needed
│     └── index.spec.jsx
│ └─┬ resources // "views"
│   └─┬ users
│     ├─┬ index
│       ├── index.css
│       ├── index.jsx
│       └── index.int.js
│     ├─┬ new
│       ├── index.css
│       ├── index.jsx // uses client/components/banner & client/components/form
│       └── index.int.js
│     ├── index.js
│     └── model.js

So the first 5 whys sound like they are just for convenient's sake and force user to remember the tests.

So is yours, except yours has me jumping round the repo 😜

[…] we also have linting rules that are different for tests.

You can use file extensions to differentiate to which files the rule configs apply: .spec.jsx and .jsx (just ensure the spec one is before the generic).

In all practicality, there are really only 2 scenarios that would actually get used:

• Co-located
• Not co-located

If they're not co-located, /<app-root>/test/client is as good of a location as any. If they are co-located, /<app-root>/client is already enforced by Electrode, so you know where to look:

var testsReq = require.context("test/client", true, /\.spec.jsx?$/);

if (!testsReq.keys().length) { // co-located
    testsReq = require.context("client", true, /\.spec.jsx?$/);
} // non-breaking

[JakobJingleheimer]

@darkbls or @jchip how does one add and register a custom Archetype? The docs tease about doing it, but never actually explain how.

I'm also having some difficulty with this. We would like a way to organize or label our integration tests, so they are easily distinguished from regular unit tests. @jshado1 following your comment above, your *.int.jsx tests would not get picked up by that require context. We were originally thinking about creating a separate folder called integration, but that would require that we create our own gulp task outside of the electrode archetype because it would not make sense to add it to test/src. Labelling the tests *.int.jsx seems like a decent solution, but they don't get picked up either.