Skip to content

Instantly share code, notes, and snippets.

@connor4312
Last active May 1, 2021 09:27
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save connor4312/73f1883d720654834b7fd40550d3b6e0 to your computer and use it in GitHub Desktop.
Save connor4312/73f1883d720654834b7fd40550d3b6e0 to your computer and use it in GitHub Desktop.
Test API

Requirements

  1. Load tests asynchronously, and gradually (pull from extension)
  2. Listen to test changes, new children, and removal of children (push from extension)
  3. Be able to place tests in the tree concurrently with a test run as results are reported (push from extension)
  4. Be able to stop listening to tests from a child
  5. Allow the extension to identity its tests in whatever "run test" mechanism there is

Approaches

Initial v1

As presented in the API call. There's a TestProvider and discoverChildren method. The TestItem is a simple class without methods or hierarchal relationships.

export interface TestProvider<T extends TestItem = TestItem> {
	/**
	* An event that should be fired whenever an existing test is updated, or
	* when a new test is discovered as a result of a {@link discoverChildren}
	* call or subsequent update. Changes to tests will not be visible until
	* this event is fired.
	*
	* It is safe to fire this event for children of TestItems that the editor
	* has not yet requested in {@link discoverChildren}.
	*/
	readonly onDidChangeTest: Event<T>;

	/**
	* An event that should be fired when an existing tests is removed.
	*/
	readonly onDidRemoveTest: Event<T>;

	/**
	 * An event that fires when a test becomes outdated, as a result of
	 * file changes, for example. In "auto run" mode, tests that are outdated
	 * will be automatically re-run after a short delay. Firing a test
	 * with children will mark the entire subtree as outdated.
	 */
	readonly onDidInvalidateTest?: Event<T>;

	/**
	 * Requets the children of the test item. When called, the provider should
	 * fire the {@link onDidUpdateTest} as it discovers tests. When all
	 * children of the provided item have been discovered, the promise should
	 * resolve.
	 *
	 * After the discovery process, the provider should continue watching for
	 * changes to the children and firing updates until the returned
	 * Disposable is disposed of. Returning the Disposable is optional, and
	 * may not be necessary in all cases. For example, if the item is a test
	 * suite in a single file, the observation of children may be handled by
	 * the file watcher.
	 *
	 * The editor will only call this method when it's interested in refreshing
	 * the children of the item, and will not call it again while there's an
	 * existing, undisposed subscription for an item.
	 *
	 * @param token Cancellation for the request. Cancellation will be
	 * requested if the test changes before the previous call completes.
	 * @returns a provider result of child test items
	 */
	discoverChildren(item: T, token: CancellationToken): ProviderResult<Disposable>;

	/**
	 * Gets the parent of the test item. This should only return "undefined"
	 * if called with the root node.
	 * @param item TestItem to retrieve the parent for
	 * @returns the parent TestItem, or undefined if the test is the root
	 */

Traditionally Managed

There was noted awkwardness around the "discoverChildren" method. It was suggested to make the object managed and have methods on it for adding children, who could later be disposed of with the dispose() method. Having an object be managed would also remove the need for the separate onDidChangeTest and onDidInvalidateTest method.

It has the drawback in making "Allow the extension to identity its tests" harder. As an unmanaged class, I can (and do) subclass it to add extra detail and meaning to each node in the tree. This could also be accomplished via some metadata: any or metadata: T field on the TestItem. However, this is harder to deal with and does not let me strongly-type the children. In the linked sample, I can type that my TestRoot only has TestFile children, and TestFiles only have TestHeaing and TestCase children.

Hybrid Managed?

I briefly brought up the idea of having tests be "magicallly managed" via some global event emitter in the extension host. At the risk of being "creative", I think something like this could actually be an elegant approach: have the TestItem class have some internal Symbol property or properties that allow the extension host to hook in after the test is first shared by the extension with the API.

Properties changes can be managed by getters/setters that tap the installed "hook", if it exists. We can also have managed methods and objects on the tests that eliminate the need for events in the TestProvider:

export class TestItem<TChildren = TestItem> {
	readonly children: Set<TChildren>; // (more likely a Set-like interface)
	invalidate(): void; // same as firing "onDidInvalidateTest"
	// ...

The hook would be installed either:

  • For the test root, at the moment that it's returned from provide*TestRoot
  • For other children, at the moment it's added to the children Set-like interface

Test Discovery

Both managed approaches don't directly solve the discovery question, but that could be done by adding the following method on TestItems, which can be optionally override if that items implements test discovery:

class TestItem {
  /**
    * Requests the children of the test item. Extensions should override this
    * method for any test that can discover children.
    *
    * When called, the item should discover tests and update its's `children`.
    * The provider will be marked as 'busy' when this method is called, and
    * the provider should report `{ busy: false }` to {@link Progress.report}
    * once discovery is complete.
    *
    * The item should continue watching for changes to the children and
    * firing updates until the token is cancelled. The process of watching
    * the tests may involve creating a file watcher, for example.
    *
    * The editor will only call this method when it's interested in refreshing
    * the children of the item, and will not call it again while there's an
    * existing, uncancelled discovery for an item.
    *
    * @param token Cancellation for the request. Cancellation will be
    * requested if the test changes before the previous call completes.
    * @returns a provider result of child test items
    */
  discoverChildren(progress: Progress<{ busy: boolean }>, token: CancellationToken): void;

We still get to eliminate the three extra events by having objects be managed, and can avoid needing to mix the Disposable and CancellationToken by using the Progress interface. This also has the benefit that the provider can signal progress during a routine, watcher-triggered update.

Also, because tests are being added to the managed collection, we no longer need a getParent method on the root, since we can enforce and require that tests be somewhere in the collection before they're reported in results.

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