{"action":"created","issue":{"url":"https://api.github.com/repos/nunit/nunit/issues/2003","repository_url":"https://api.github.com/repos/nunit/nunit","labels_url":"https://api.github.com/repos/nunit/nunit/issues/2003/labels{/name}","comments_url":"https://api.github.com/repos/nunit/nunit/issues/2003/comments","events_url":"https://api.github.com/repos/nunit/nunit/issues/2003/events","html_url":"https://github.com/nunit/nunit/issues/2003","id":202401392,"number":2003,"title":"Better user info about ParallelizableAttribute and ParallelScope","user":{"login":"CharliePoole","id":2678661,"avatar_url":"https://avatars.githubusercontent.com/u/2678661?v=3","gravatar_id":"","url":"https://api.github.com/users/CharliePoole","html_url":"https://github.com/CharliePoole","followers_url":"https://api.github.com/users/CharliePoole/followers","following_url":"https://api.github.com/users/CharliePoole/following{/other_user}","gists_url":"https://api.github.com/users/CharliePoole/gists{/gist_id}","starred_url":"https://api.github.com/users/CharliePoole/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CharliePoole/subscriptions","organizations_url":"https://api.github.com/users/CharliePoole/orgs","repos_url":"https://api.github.com/users/CharliePoole/repos","events_url":"https://api.github.com/users/CharliePoole/events{/privacy}","received_events_url":"https://api.github.com/users/CharliePoole/received_events","type":"User","site_admin":false},"labels":[{"id":61608516,"url":"https://api.github.com/repos/nunit/nunit/labels/is:enhancement","name":"is:enhancement","color":"84b6eb","default":false},{"id":61616865,"url":"https://api.github.com/repos/nunit/nunit/labels/pri:normal","name":"pri:normal","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":{"login":"CharliePoole","id":2678661,"avatar_url":"https://avatars.githubusercontent.com/u/2678661?v=3","gravatar_id":"","url":"https://api.github.com/users/CharliePoole","html_url":"https://github.com/CharliePoole","followers_url":"https://api.github.com/users/CharliePoole/followers","following_url":"https://api.github.com/users/CharliePoole/following{/other_user}","gists_url":"https://api.github.com/users/CharliePoole/gists{/gist_id}","starred_url":"https://api.github.com/users/CharliePoole/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CharliePoole/subscriptions","organizations_url":"https://api.github.com/users/CharliePoole/orgs","repos_url":"https://api.github.com/users/CharliePoole/repos","events_url":"https://api.github.com/users/CharliePoole/events{/privacy}","received_events_url":"https://api.github.com/users/CharliePoole/received_events","type":"User","site_admin":false},"assignees":[{"login":"CharliePoole","id":2678661,"avatar_url":"https://avatars.githubusercontent.com/u/2678661?v=3","gravatar_id":"","url":"https://api.github.com/users/CharliePoole","html_url":"https://github.com/CharliePoole","followers_url":"https://api.github.com/users/CharliePoole/followers","following_url":"https://api.github.com/users/CharliePoole/following{/other_user}","gists_url":"https://api.github.com/users/CharliePoole/gists{/gist_id}","starred_url":"https://api.github.com/users/CharliePoole/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CharliePoole/subscriptions","organizations_url":"https://api.github.com/users/CharliePoole/orgs","repos_url":"https://api.github.com/users/CharliePoole/repos","events_url":"https://api.github.com/users/CharliePoole/events{/privacy}","received_events_url":"https://api.github.com/users/CharliePoole/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2017-01-22T18:51:17Z","updated_at":"2017-01-22T20:36:45Z","closed_at":null,"body":"This comes up as I work on issue #164 and I'll implement it in conjunction with that is.sue. However, I think it needs to have separate review, which is why I'm creating this issue.\r\n\r\n`ParallelizableAttribute` takes an optional argument of Type `ParallelScope`. The values of `ParallelScope` are a mix of things. Some apply to the item on which the attribute appears and others to descendants. In addition, `ParallelScope.None` is used both when specified by the user and as a default. This turns out to lose info for us, since there are three, rather than two, actions the dispatcher may take in running a test:\r\n * Run directly on the same thread as parent\r\n * Enqueue on a parallel queue\r\n * Enqueue on a non-parallel queue\r\n\r\nI'm proposing a number of changes to make parallel test cases work and also to clarify things for users:\r\n\r\n1. I have already defined `ParallelScope.Default` which indicates that no attribute was used. This turned out to be necessary to effectively implement parallel test cases.\r\n\r\n2. For convenience internally I have re-defined the various scopes in two ranges, separating those flags that affect the item on which they appear from those that affect descendants. I use two masks to pick out the flags that are needed in a particular instance.\r\n\r\n[Remaining items have to do with how this appears to users]\r\n\r\n3. I'd like to deprecate `ParallelScope.None` in favor of `ParallelScope.NonParallel`. The former name is confusing enough that it was confusing me in working #164. Its implementation has always been \"run test on non-parallel queue\" but it sounds as if it means \"not specified\" or \"use default\". I'm open to other names. I considered \"Never\" as an option.\r\n\r\n4. For consistency, I'd like to define `ParallelScope.Parallel` as a synonym for `ParallelScope.Self`. I wouldn't deprecate the latter, however, because combos like `ParallelScope.Self+ParallelScope.Children` read quite well.\r\n\r\n5. To make it even easier for users. It may be useful to define `NonParallelizableAttribute` with no arguments. Just as `[Parallelizable]` defaults to `[Parallelizable(ParallelScope.Self)`, this new attribute would mean the same as `[Parallelizable(ParallelScope.NonParallel)`\r\n\r\n6. Some values of `ParallelScope` make no sense in certain places and are currently ignored silently. For example, `Parallelizable(ParallelScope.Fixtures)` has no effect if placed on a test case. Rather than silently ignoring the attribute, I propose that we mark the test as invalid so that the user knows it won't do whatever is expected and removes it. Implementation will require looking at each scope value and deciding whether to allow it on an Assembly, a class or a method. It seems to me that getting an early error is better for the user than having their intent (which we don't know of course) ignored silently.\r\n\r\n7. I propose adding `ParallelScope.Cases` to the set of scopes that impact descendants. Used at the assembly level without combining with other scopes, it would cause test fixtures to run sequentially but all the cases within them to run in parallel. I don't think this will get a lot of use but it's consistent with the presense of `ParallelScope.Fixtures`. I would redefine `Children` as `Fixtures+Cases` if we do this.\r\n\r\nPlease comment! For implementation details, you'll be able to review the PR of course."},"comment":{"url":"https://api.github.com/repos/nunit/nunit/issues/comments/274357346","html_url":"https://github.com/nunit/nunit/issues/2003#issuecomment-274357346","issue_url":"https://api.github.com/repos/nunit/nunit/issues/2003","id":274357346,"user":{"login":"CharliePoole","id":2678661,"avatar_url":"https://avatars.githubusercontent.com/u/2678661?v=3","gravatar_id":"","url":"https://api.github.com/users/CharliePoole","html_url":"https://github.com/CharliePoole","followers_url":"https://api.github.com/users/CharliePoole/followers","following_url":"https://api.github.com/users/CharliePoole/following{/other_user}","gists_url":"https://api.github.com/users/CharliePoole/gists{/gist_id}","starred_url":"https://api.github.com/users/CharliePoole/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CharliePoole/subscriptions","organizations_url":"https://api.github.com/users/CharliePoole/orgs","repos_url":"https://api.github.com/users/CharliePoole/repos","events_url":"https://api.github.com/users/CharliePoole/events{/privacy}","received_events_url":"https://api.github.com/users/CharliePoole/received_events","type":"User","site_admin":false},"created_at":"2017-01-22T20:36:45Z","updated_at":"2017-01-22T20:36:45Z","body":"Well, if we just rename, they will get a compiler error. If we mark as obsolete, they will get a compiler warning with a message that tells them to use `NonParallel`. Both require changing the code but deprecation seems slightly more user-friendly and you can ignore the message if you like. What do others think?\r\n\r\n`Cases` was just an idea we can easily drop. The only functionality it adds is when it is used on the assembly, which I don't think would be very common. On a fixture, it would be equivalent to `Children` since we don't (currently) treat nested classes as children of the surrounding class."}}
{"actor":{"display_login":"CharliePoole"}}