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 integration tests for package dependencies #1144

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michael-weng
Copy link
Contributor

  • Verify various contract calls with spm
  • Exercises use local version workflow

Issue: #1052

- Verify various contract calls with spm
- Exercises use local version workflow

Issue: swiftlang#1052
expect(output).to.include("required");

// Contract: spm reset, resolve, update should work
const executeTaskSpy = sinon.spy(utilities, "executeTaskWithUI");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test gets into needing some intimate knowledge of the implementation for spying / mocking. If this is tied to a command, we should execute the command. Instead of mocking the user selection, you can let the command take a parameter and pass that parameter when running executeCommand

test("Use local dependency", async () => {
// Expect to fail without setting up local version
const tasks = (await getBuildAllTask(folderContext)) as SwiftTask;
let { exitCode, output } = await executeTaskAndWaitForResult(tasks);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably could move this to a setup hook so it is separate from test purpose

const updateWorkspaceSpy = sinon.spy(vscode.workspace, "updateWorkspaceFolders");
// We would love to call uneditDependency for coverage but there's no clean way to get
// a synchronize point for deterministic task completion so just call this function direct
await uneditFolderDependency(workspaceContext.currentFolder!, id, workspaceContext);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"full workflow", i.e. multiple paths and actions, is more smoke level. Find to group a feature in a suite, but > let's make sure each test case is one path.

This is sort of what I meant. I'm not quite sure what we're testing here. We're checking exit codes above, further actions and assertions in subsequent steps. Ideally 1 action, 1 result, 1 set of assertion(s) on that result for each test case

expect(file).to.not.be.undefined;

expect(file?.path).to.equal(`${path}/Sources/CAtomic/CAtomic.c`);
suite("Full work flow tests", function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"full workflow", i.e. multiple paths and actions, is more smoke level. Find to group a feature in a suite, but let's make sure each test case is one path.

// FIXME: Can be changed back to Swift-Markdown when
// https://github.com/swiftlang/swift-package-manager/issues/7931
// is released in the toolchain
name: "swift-markdown",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this arbitrary or must it be "swift-markdown" because we want it to overshadow the remote dependency? If the latter, should have a comment "needs to be swift-markdown for .... test to verify ...."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right this is not arbitrary, I will add comment to make this more obvious.

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