L1 Testing

[damccorm]

This enables writing L1 functional tests for the worker. Full design here:

Agent L1 Testing

Writing an L1 Test

All L1 tests extend the L1TestBase class. This class provides utilities for mocking HTTP classes, loading job messages, starting the worker, and fetching data from the mocks.

Example L1 test:

public class WorkerL1Tests : L1TestBase
{
    [Fact]
    [Trait("Level", "L1")]
    [Trait("Category", "Worker")]
    public async Task ExampleTest()
    {
        // Arrange
        var message = LoadTemplateMessage();

        // Act
        var results = await RunWorker(message);

        // Assert
        AssertJobCompleted();
        Assert.Equal(TaskResult.Succeeded, results.Result);

        var steps = GetSteps();
        Assert.Equal(4, steps.Count());
        Assert.Equal(1, steps.Where(x => x.Name == "CmdLine").Count());
    }
}

Setup

LoadTemplateMessage() loads a base AgentJobRequestMessage. This is the object that the server sends down to the agent that tells it all the information it needs to run a job. This includes steps, variables, resources, and service connections.

This base message can be modified in code to add new steps, change variables, etc.

Run

RunWorker() starts the worker with the given message. This will run all steps in the message to completion. The return value includes the result of the job and any exceptions it encountered.

Assert

AssertJobCompleted() is a util method to verify the job sent a JobCompletedEvent()
GetSteps() is a util function that fetches TimelineRecords from the mocks and returns all steps executed during the course of the job.
GetMockedService<T>() returns any of the services mocked in the L1TestBase class. This allows test writers to manually assert values recorded by the HTTP mock classes.

Design

Goals

The agent L1 tests should allow us to execute the full code path for the worker, starting at receiving a job message and ending at reporting results. L1 tests should be able to run offline, meaning any outgoing requests to the service or to source control need to be mocked.

Test writers should be able to assert against data sent through outgoing requests, so we will need to intercept any outgoing requests in the mocks and record what they are doing.

Tasks should still run normally in their own process. We won't download them from the service though.

L1 tests should be easy to write for new features (<30 minutes).

How the tests will run

Tests will invoke the worker directly from a L1 base class via RunWorker(). That means the worker itself will run inside the test process. This gives us advantages over running the Agent.Worker.exe from the test since we can more easily inject mocks and can access data from the mocked classes. This will reduce the time it takes for authors to write L1 tests while still meeting out proposed goals (testing the full codepath).

RunWorker() vs Agent.Worker.exe

During design discussion, we debated between having the test methods spawn their own worker process or having them invoke the worker directly. In the end, we decided the benefits of invoking directly outweigh the small improvements in test completeness that come with running the worker in a separate process. If the goal of the tests is to execute the full codepath of the worker project, this can be acomplished entirely by both methods. Therefore, the added complexity of running a separate process and coordinating communication between the test process and the worker process to pass data like TimelineRecord or JobCompletedEvent objects was not worth the cost.

Tasks

Since we won't be downloading tasks from the service, we need to include a few test tasks that test writers can invoke.
We have decided on including a generic node task and a powershell task in the L1 test folder that will be zipped and sent to the worker as part of the test run. This will allow test writers to use those tasks in normal ways (including calling ##vso commands) to verify functionality.
More tasks can also be added by test writers if necessary.

Job messages

The job message will have a base JSON template available for test writers. This template was taken from the _diag logs from an actual build, so it includes the full set of variables and resoruces. Test writers will have the option of extending this template by loading the AgentJobRequestMessage and manipuating it by adding steps, changing varaibles, etc. If the base template is not sufficient, test writers can also include their own JSON job message blobs in their test classes which they can load using the parser.

[stephenmichaelf]

I am going through this sequentially, do we need to derive from these classes? Feels like a code smell.

[damccorm]

do we need to derive from these classes?

I think so. The alternative would be to derive directly from AuthenticatedGitSourceProvider and provide overrides for the specific properties/methods (e.g. GitSupportsFetchingCommitBySha1Hash). That feels more likely to drift out of sync

Feels like a code smell.

Not sure what that means

[stephenmichaelf]

Personal preference would be to have a class like TestingStringUtil in the test project with this method instead of adding it to non testing code if we only need it for testing.

Having it here implies it's got production quality tests and some use outside of testing.

[stephenmichaelf]

On second look we are setting some values on the util here.

This setup feels wrong to me :) It may be out of the scope of the PR but feels like it's structured incorrectly.

Maybe also this class should be split into 2? One for StringUtil and one for LocalizationStringUtil?

[alex-peck]

This class might be worth revisiting in the future, but it seems out of scope here. With the current design, this seems like the best way to achieve the objective of getting some localization loaded for testing.

[stephenmichaelf]

Same question about deriving.

[damccorm]

Not sure which question this is in reference to

[damccorm]

We want to derive here so that we can add/remove plugins without altering any of the rest of the execution

[stephenmichaelf]

Same question about types. I can't tell from this line the type of artifact.

[stephenmichaelf]

Do we need the when?

[alex-peck]

This code was copied from the other IAgentService classes that use a VssConnection. From what it looks like, the when isn't necessary here, but I'd rather not change that here in this PR without changing other places with this pattern.

[stephenmichaelf]

Same question about need to derive from class.

[damccorm]

Still not sure what the question is, but we need this so that we can override the Load method without touching any of the rest of the functionality

[stephenmichaelf]

When do we not want to dispose?

[alex-peck]

The tests act as a client and server in the same process. We need avoid disposing the client handle since handles are per-process.

These get disposed properly when they go out of scope (see Dispose() in this class)