qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
60 stars 18 forks source link

Constants file #55

Closed flore77 closed 8 years ago

flore77 commented 8 years ago

Need

We need a constants file so that we can quickly change values in the entire project.

Deliverables

A file that will contain all our constants, like: events names, test statuses etc.

Solution

Create a file that exports multiple objects like:

const events = {
  runStart: 'runtStart',
  ...
}

const testStatuses = {
  passing: 'passed'
  ...
}

export events
export testStatus

We can also use keymirror:

const events = keyMirror({
  runStart: null,
  ...
})

And replace the strings with the the defined constants in the whole project.

flore77 commented 8 years ago

What do you say @jzaefferer ? I personally find it very useful, because if someday we decide that testStart event should be renamed in testOnStart, it wouldn't be so nice, to search it in the whole project to replace it.

jzaefferer commented 8 years ago

I'm not sure what problem this is supposed to solve. Changing testStart to testOnStart would be a breaking change and shouldn't be easy. At the same time, doing a search-and-replace isn't complicated.

Replacing strings with identifiers that have the same value as the string doesn't add any value. If the strings don't match the identifiers, they become confusing instead.

flore77 commented 8 years ago

Yes, but there are strings that we use across multiple files and I thought it would be good to store them into constants variables and to use the variables instead.

Yes, it would be a breaking change in the standard, but it would be really easy to implement, for example for the tests statuses if you think that it would be better to be: skip, pass and fail. You should do the changes in only one file. This would give also a better overview over the standard, without checking the description of the standard, this is why I was thinking it would be good.

jzaefferer commented 8 years ago

Yes, but there are strings that we use across multiple files and I thought it would be good to store them into constants variables and to use the variables instead.

I remember doing just that, also for event names, on a project some years ago, then later realized that it was a bad idea and reverted it all. As far as I remember, the abstraction didn't add any value, it just made things more error-prone.

I'm fine with looking for other abstractions that fulfil the other aspects you described, but this isn't it.

flore77 commented 8 years ago

ok :smiley: