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

Add environment setting and detection. Add 'run tests' command #372

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Willyham
Copy link

@Willyham Willyham commented Nov 2, 2022

This adds a developmentEnvironment configuration setting, with options (none, forge, hardhat). When the extension activates it attempts to automatically set this environment if it's not set.

When the environment config value changes, we attempt to set specific environment based configuration, if those are not already set. The only environment currently configured with any defaults or behaviour is forge.

It also adds a runTests command. When invoked, this runs the value in the solidity.test.command configuration setting, and attempts to interpret the output of the command based on the configured environment.

Screen.Recording.2022-11-02.at.12.43.57.mov

I'll follow this PR up with more features:

  • Automatically running test when a file is saved
  • Support for test coverage

@juanfranblanco
Copy link
Owner

Hi @Willyham this needs refactoring a bit, as the testing is getting mixed with environment, and the command executed directly on the extension. I can do that, but that may break your other changes.

@Willyham
Copy link
Author

Willyham commented Nov 4, 2022

Hey, I'm happy to fix things up if needed. Not quite sure what you mean about getting mixed with the environment?

In terms of the command execution, I wasn't really sure about the extension architecture and the separation of the extension and the server. Does that need to be executed on the server and the data passed back to the extension? Is there any docs you can point me towards which explains the 'why' for that? Thanks!

@juanfranblanco
Copy link
Owner

No. this does not need to be in the server side, just the client. Although it could be but it will complicate things for the time being.
Re: structure

Environment
Client (as it is not going to be used in the server) -> Environment -> EnvironmentAutoConfiguration.ts (or better naming)

if (!event.affectsConfiguration('solidity.developmentEnvironment')) {
return;
}
const newConfig = vscode.workspace.getConfiguration('solidity');
const newEnv = newConfig.get<DevelopmentEnvironment>('developmentEnvironment');
const vaulesToSet = defaultEnvironmentConfiguration[newEnv];
if (!vaulesToSet) {
return;
}
for (const [k, v] of Object.entries(vaulesToSet)) {
if (newConfig.get(k) === v.default) {
newConfig.update(k, v.target, null);
}
}

// Detect development environment if the configuration is not already set.

const detectDevelopmentEnvironment = async (): Promise<string> => {
const foundry = await workspace.findFiles('foundry.toml');
const hardhat = await workspace.findFiles('hardhat.config.js');
// If we found evidence of multiple workspaces, don't select a default.
if (foundry.length && hardhat.length) {
return DevelopmentEnvironment.None;
}
if (foundry.length) {
return DevelopmentEnvironment.Forge;
}
if (hardhat.length) {
return DevelopmentEnvironment.Hardhat;
}
return DevelopmentEnvironment.None;
};

This could also have "truffle" whilst at it and dapptools

Task execution
Common Utils

const executeTask = (dir: string, cmd: string, rejectOnFailure: boolean) => {
return new Promise<string>((resolve, reject) => {
cp.exec(cmd, {cwd: dir, maxBuffer: 1024 * 1024 * 10}, (err, out) => {
if (err) {
if (rejectOnFailure) {
return reject({out, err});
}
return resolve(out);
}
return resolve(out);
});
});
};

to here.. https://github.com/juanfranblanco/vscode-solidity/blob/master/src/common/util.ts

Rootfolder
Already here:
https://github.com/juanfranblanco/vscode-solidity/blob/master/src/client/workspaceUtil.ts#L8

@juanfranblanco
Copy link
Owner

Tests
Client (as it is not going to be used in the server) -> Tests

  • tests file https://github.com/juanfranblanco/vscode-solidity/pull/372/files#diff-4b9bcfa5bddab1f7e8138380cb43399f41d7b34cb980a496d03b051028fe614c
  • forge file.
  • and this section can be moved to the tests file
    const testCommand = vscode.workspace.getConfiguration('solidity').get<string>('test.command');
    if (!testCommand) {
    return;
    }
    if (!testOutputChannel) {
    testOutputChannel = vscode.window.createOutputChannel('Solidity Tests');
    }
    // If no URI supplied to task, use the current active editor.
    let uri = params?.uri;
    if (!uri) {
    const editor = vscode.window.activeTextEditor;
    if (editor && editor.document) {
    uri = editor.document.uri;
    }
    }
    const rootFolder = getFileRootPath(uri);
    if (!rootFolder) {
    console.error("Couldn't determine root folder for document", {uri});
    return;
    }
    testOutputChannel.clear();
    testOutputChannel.show();
    testOutputChannel.appendLine(`Running '${testCommand}'...`);
    testOutputChannel.appendLine('');
    try {
    const result = await executeTask(rootFolder, testCommand, false);
    const parsed = parseTestResults(result);
    // If we couldn't parse the output, just write it to the window.
    if (!parsed) {
    testOutputChannel.appendLine(result);
    return;
    }
    const out = constructTestResultOutput(parsed);
    out.forEach(testOutputChannel.appendLine);
    } catch (err) {
    console.log('Unexpected error running tests:', err);
    }
    ExecuteTests?

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

Successfully merging this pull request may close these issues.

2 participants