Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolver based discovery mechanism #134

Closed
wants to merge 54 commits into from

Conversation

bechte
Copy link
Contributor

@bechte bechte commented Jan 24, 2016

This pull request contains all changes that are required to switch to another discovery process. The new process is built upon resolvers and follows a clear structure for the unique id. It encapsulates all logic that is required to create and map a TestDescriptor with an guaranteed unique id within the TestResolver.

Advantages over the current discovery process:

  • No hard-coded rules for test discovery: The current approach needs to be changed in code in order to add new TestDescriptors within the test plan tree. The new approach allows to add children to a given parent any time during the test discovery phase. In this way, extension provider could also write their own TestResolvers that can parse arbitrary syntax trees (e.g. lambda, XML files, etc.)
  • the concept of a TestResolver simplifies the dependencies: A TestResolver has no dependency on other TestResolvers. It only knows about the TestDescriptor that it want to contribute new children to, and knows how to describe them in form of a DiscoverySelector. In this way, discovery can be done top-down (recursively) or bottom-up (exact match only).
  • it provides better maintainability by adding a clear separation of concerns: A TestResolver is responsible for creating the executable TestDescriptors and to resolve the UniqueId for it. It needs not to know about any other TestResolver in the system, but only about which TestDescriptor it wants to add children to.
  • Concentration of rules for UniqueIds: TestResolvers are responsible for creating and resolving UniqueIds. No other module will need to contain rules for modifying or changing an UniqueId.
  • easier integration of DiscoveryFilter: Each TestResolver might provide its own DiscoveryFilters that it will apply during discovery. This gains performance, since a DiscoveryFilter is applied upfront by its TestResolver and no TestDescriptor needs to be generated, at all. DiscoveryFilter might be understood as 'Tuning or Configuration parameters' for the TestResolvers

Team decision: Everybody should review this pull request until the next team Skype call on Tuesday. Then we will decide whether or not we will switch to the new discovery process.

@codecov-io
Copy link

Current coverage is 86.02%

Merging #134 into master will decrease coverage by -0.45% as of 71b295d

@@            master    #134   diff @@
======================================
  Files          142     151     +9
  Stmts         2964    3384   +420
  Branches       304     371    +67
  Methods          0       0       
======================================
+ Hit           2563    2911   +348
- Partial         90     103    +13
- Missed         311     370    +59

Review entire Coverage Diff as of 71b295d

Powered by Codecov. Updated on successful CI builds.

@bechte bechte force-pushed the feature-107-discoveryresolver branch from be0f866 to a132740 Compare January 24, 2016 13:09
bechte added 26 commits January 27, 2016 09:08
(needs to be restricted in the next step)
@bechte bechte force-pushed the feature-107-discoveryresolver branch from edc4ab8 to 71b295d Compare January 27, 2016 08:21
@jlink jlink mentioned this pull request Mar 31, 2016
bechte added a commit that referenced this pull request Apr 29, 2016
Team decision:
- We choose to take this pull request instead of #134 to be able to ship M1 faster.
- Therefore, #134 will be closed and not merged
- I will open a new issue for extending the behaviour according to #134 that we can work on in a later milestone
@bechte
Copy link
Contributor Author

bechte commented Apr 29, 2016

Team decision:

  • We choose to take pull request New discovery mechanism #218 instead of this one to be able to ship M1 faster.
  • Therefore, this PR will be closed and not merged
  • I will open a new issue for extending the behaviour according to this PR that we can work on in a later milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants