Deno: Refactor stringified Perm from unit test

Created on 12 Apr 2019  路  8Comments  路  Source: denoland/deno

as mentionned in: https://github.com/denoland/deno/pull/1977

As an aside (for another time), I feel like r----- rw---- etc would be a little clearer/more concise here. (perhaps rwxent?)

cc @ry @hayd

I'll give it a purpose for this.

Most helpful comment

what about (instead of unix file permissions style) you use no-perms vs perms-rw perms-ne etc. I find the current scheme very hard to read; most of the time you can about what permission is present not what permission is not present, hence perms-rw over perms_R1W1N0E0H0.
perms-RWneh is also an alternative but probably too cute.

This is minor to the main refactoring issue.

All 8 comments

I think those strings are less readable than the current scheme. Ideally a unit_test refactoring would do this:

  1. load the unit tests without running them, get a list of all the permissions
  2. automatically start deno subprocesses which filter on all permission combinations present in the list from 1.

Just to be clear, the python script will load a array of declarations of .ts files to execute with the permissions associated. After it runs each .ts file with its the associated permission. Is that how you want to implement it?

what about (instead of unix file permissions style) you use no-perms vs perms-rw perms-ne etc. I find the current scheme very hard to read; most of the time you can about what permission is present not what permission is not present, hence perms-rw over perms_R1W1N0E0H0.
perms-RWneh is also an alternative but probably too cute.

This is minor to the main refactoring issue.

I vote for perms-xxx

@zekth @hayd I mean - the scheme of using command-line arguments to filter the tests isn't necessarily necessary. What if we start js/unit_tests.ts with full permissions and use revokePermission()? I'm not sure that works - but I'm suggesting that we think outside the box, and refactor this entire system, rather than bike shedding on how to represent the test filter in a string.

A helpful first step towards this (and to better understand the problem) would be to port tools/unit_tests.py to Deno.

Just want to note that the poor design of tools/unit_tests.py has already caused the networking tests to be silently disabled: https://github.com/denoland/deno/issues/2267

We need to refactor it soon.

I'll do it during weekend, I think we can just combine every possible flag without manually specifying perm flags

@zekth we can close this issue now

Was this page helpful?
0 / 5 - 0 ratings

Related issues

davidbarratt picture davidbarratt  路  3Comments

ry picture ry  路  3Comments

doutchnugget picture doutchnugget  路  3Comments

metakeule picture metakeule  路  3Comments

ry picture ry  路  3Comments