proper icon indicating copy to clipboard operation
proper copied to clipboard

node type supported

Open gliush opened this issue 12 years ago • 2 comments

I needed a proper test for the module that works with the 'node()' type. I'm not sure that it's done correctly as I don't know the code good enough. I've got the 'bool()' type as an example for my modification.

btw, I don't quite understand why in a comment in proper_types.erl it's said that 'node' type won't be done.

gliush avatar Apr 25 '13 18:04 gliush

The node() type is supposed to represent valid node (i.e. machine) names, that is they are supposed to be atoms of the form '[email protected]', not random atoms such as 'foo#42b!&!ar'.

So, in most test case scenarios that we envisioned, the generation of valid node names has to be test-environment specific (e.g. the user has to supply a valid list of machine and domain names) in order for the test to be properly executed. Hence, it cannot be totally random.

Is your use-case different than this? Perhaps you can give us some more info why declaring node() as just atom() is something that makes sense in all cases. Otherwise, we cannot merge your pull request.

kostis avatar Apr 28 '13 18:04 kostis

Agreed with the environment specific nature of node(). But not everybody need it. And moreover absence of any node() generator hurts more, than it's simplest realization.

About my use-case: I have an internal config handler and processor. It doesn't have functions with side-effects, just saving and processing input data. It has several records specifications, in one of them node() type is used. As that recors is used everywhere throughout the module, I can't check_specs any function in my module. So I don't need any node-specific logic there, an atom generator perfectly fits it.

I had two options for the check_specs to work: either use atom() instead of node() or have a node() generator, semantically equal to atom(). As I want the code to be self-documented, I don't want to use the first one.

We could consider adding more logic to the node() generator. I'd propose not supporting long node names, as I don't see an easy way to support both short and long node names (I mean generator shouldn't mix them, so the user should somehow specify how what should proper generate, and I don't see how it could be done).

And short node name should be generated with the usual restrictions to the node names.

What do you think?

gliush avatar Apr 29 '13 16:04 gliush