Skip to content

fix test workflow #771

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 24, 2025
Merged

fix test workflow #771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da0a506
Add build-vscode command
vezwork Jul 24, 2025
333cdb5
Disable test skipping in CI to diagnose issue
vezwork Jul 24, 2025
3c70e17
Remove test skips in CI!
vezwork Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ jobs:

- name: Build vscode extension
run: |
cd apps/vscode
yarn install
yarn run build
yarn run build-vscode

- name: Compile and run tests
run: |
Expand Down
2 changes: 1 addition & 1 deletion apps/vscode/.vscode-test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default defineConfig([
files: 'test-out/*.test.js',
workspaceFolder: 'src/test/examples',
mocha: {
timeout: 3000,
timeout: 5000,
},
},
]);
14 changes: 6 additions & 8 deletions apps/vscode/src/test/quartoDoc.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { exampleWorkspacePath, exampleWorkspaceOutPath, copyFile, wait } from ".
import { isQuartoDoc } from "../core/doc";
import { extension } from "./extension";

const APPROX_TIME_TO_OPEN_VISUAL_EDITOR = 1600;
const APPROX_TIME_TO_OPEN_VISUAL_EDITOR = 1700;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, I believe you, but if we're cutting this short, then maybe we should bump it to a notably larger number?

I'm not sure that a 100/1600 ~ 6% increase is justifiable without a deeper understanding as to why the number would be consistently larger than 1600ms but consistently smaller than 1700.

Copy link
Collaborator Author

@vezwork vezwork Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

All I know is that I originally tried an additional await wait(APPROX_TIME_TO_OPEN_VISUAL_EDITOR after await extension().activate() and the tests passed in CI, then I removed that additional wait and the roundtripping test failed, then I adjusted this from 1600 to 1700 on the feeling that a bit of extra waiting seems to help and the tests all passed.

Altogether it is unclear what is happening. Extra waiting does seem to help. Its unclear where that waiting should be but it seems that the waiting does not have to be before switching to the visual editor.

Copy link
Collaborator Author

@vezwork vezwork Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I expect this may take some finessing once we have seen more. I wouldn't be surprised if the tests flake on other PRs or once more tests are added or anything changes.


suite("Quarto basics", function () {
// Before we run any tests, we should copy any files that get edited in the tests to file under `exampleWorkspaceOutPath`
Expand All @@ -23,12 +23,11 @@ suite("Quarto basics", function () {

// Note: the following tests may be flaky. They rely on waiting estimated amounts of time for commands to complete.
test("Can edit in visual mode", async function () {
// don't run this in CI for now because we haven't figured out how to get the LSP to start
if (process.env['CI']) this.skip();

const doc = await vscode.workspace.openTextDocument(exampleWorkspaceOutPath("hello.qmd"));
const editor = await vscode.window.showTextDocument(doc);

await extension().activate();

// manually confirm visual mode so dialogue pop-up doesn't show because dialogues cause test errors
// and switch to visual editor
await vscode.commands.executeCommand("quarto.test_setkVisualModeConfirmedTrue");
Expand All @@ -42,12 +41,11 @@ suite("Quarto basics", function () {
// test. That's okay for this test, but could cause issues if you expect a qmd to look how it
// does in `/examples`.
test("Roundtrip doesn't change hello.qmd", async function () {
// don't run this in CI for now because we haven't figured out how to get the LSP to start
if (process.env['CI']) this.skip();

const doc = await vscode.workspace.openTextDocument(exampleWorkspaceOutPath("hello.qmd"));
const editor = await vscode.window.showTextDocument(doc);

await extension().activate();

const docTextBefore = doc.getText();

// switch to visual editor and back
Expand All @@ -59,6 +57,6 @@ suite("Quarto basics", function () {
await wait(300);

const docTextAfter = doc.getText();
assert.ok(docTextBefore === docTextAfter);
assert.ok(docTextBefore === docTextAfter, docTextAfter);
});
});
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"build": "turbo run build",
"dev-writer": "turbo run dev --filter writer*",
"dev-vscode": "turbo run dev --filter quarto...",
"build-vscode": "turbo run build --filter quarto...",
"test-vscode": "cd apps/vscode && yarn test",
"install-vscode": "cd apps/vscode && yarn install-vscode",
"install-positron": "cd apps/vscode && yarn install-positron",
Expand Down