move giscus smoke tests to quarto-dev/quarto-cli

[gordonwoodhull]

Ref #13298

These seem to be just smoke tests and they only need a valid, giscus-enabled repo.

I'm not clear how these are tested. The repo change in the configuration makes them preview ok!

PR to see if this clears CI.

Draft because I don't understand how this is tested!

[posit-snyk-bot]

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

[cderv]

I must admit that I don't know either... I can't find back the CI log with the issue, and I can't reproduce anymore. So I don't know really what those tests do or why they would fail... 🤔

[cderv]

And I found back the error I have seen

[smoke] > quarto render docs\smoke-all\issues\4820-giscus-dark-mode\lightly-light-cobalt\index.qmd --to html ... FAILED (21s)
Uncaught error from ./smoke/smoke-all.test.ts FAILED

 ERRORS 

[smoke] > quarto render docs\smoke-all\issues\4820-giscus-dark-mode\lightly-light-cobalt\index.qmd --to html => ./test.ts:329:8
error: AssertionError: Failed assertion: 

--------------------------------------------------------------------------------
[smoke] > quarto render docs\smoke-all\issues\4820-giscus-dark-mode\lightly-light-cobalt\index.qmd --to html
          run-tests.ps1 smoke\smoke-all.test.ts

[verify] > No Errors or Warnings

Error or Warnings During Execution
|TypeError: error sending request for url (https://giscus.app/api/discussions/categories?repo=allenmanning/giscus-examples): client error (Connect): tcp connect error: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060): A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060)

Stack trace:
    at async mainFetch (ext:deno_fetch/26_fetch.js:170:12)
    at async fetch (ext:deno_fetch/26_fetch.js:392:7)
    at async getGithubDiscussionsMetadata (file:///D:/a/quarto-cli/quarto-cli/src/core/giscus.ts:38:20)
    at async htmlFormatExtras (file:///D:/a/quarto-cli/quarto-cli/src/format/html/format-html.ts:589:30)
    at async Object.formatExtras (file:///D:/a/quarto-cli/quarto-cli/src/format/html/format-html.ts:207:11)
    at async runPandoc (file:///D:/a/quarto-cli/quarto-cli/src/command/render/pandoc.ts:526:10)
    at async renderPandoc (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render.ts:202:24)
    at async Object.onRender (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:720:30)
    at async renderFileInternal (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:682:9)
    at async renderFiles (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:330:9)|
AssertionError: Error or Warnings During Execution
|TypeError: error sending request for url (https://giscus.app/api/discussions/categories?repo=allenmanning/giscus-examples): client error (Connect): tcp connect error: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060): A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060)

Stack trace:
    at async mainFetch (ext:deno_fetch/26_fetch.js:170:12)
    at async fetch (ext:deno_fetch/26_fetch.js:392:7)
    at async getGithubDiscussionsMetadata (file:///D:/a/quarto-cli/quarto-cli/src/core/giscus.ts:38:20)
    at async htmlFormatExtras (file:///D:/a/quarto-cli/quarto-cli/src/format/html/format-html.ts:589:30)
    at async Object.formatExtras (file:///D:/a/quarto-cli/quarto-cli/src/format/html/format-html.ts:207:11)
    at async runPandoc (file:///D:/a/quarto-cli/quarto-cli/src/command/render/pandoc.ts:526:10)
    at async renderPandoc (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render.ts:202:24)
    at async Object.onRender (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:720:30)
    at async renderFileInternal (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:682:9)
    at async renderFiles (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:330:9)|
    at Module.assert (https://jsr.io/@std/assert/0.224.0/assert.ts:18:11)
    at assert (https://jsr.io/@std/testing/0.224.0/asserts.ts:608:11)
    at Object.verify (file:///D:/a/quarto-cli/quarto-cli/tests/verify.ts:166:7)
    at fn (file:///D:/a/quarto-cli/quarto-cli/tests/test.ts:240:25)
    at eventLoopTick (ext:core/01_core.js:175:7)
    at async innerWrapped (ext:cli/40_test.js:191:5)
    at async outerWrapped (ext:cli/40_test.js:134:14)

OUTPUT:
    TypeError: error sending request for url (https://giscus.app/api/discussions/categories?repo=allenmanning/giscus-examples): client error (Connect): tcp connect error: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060): A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060)
    
    Stack trace:
        at async mainFetch (ext:deno_fetch/26_fetch.js:170:12)
        at async fetch (ext:deno_fetch/26_fetch.js:392:7)
        at async getGithubDiscussionsMetadata (file:///D:/a/quarto-cli/quarto-cli/src/core/giscus.ts:38:20)
        at async htmlFormatExtras (file:///D:/a/quarto-cli/quarto-cli/src/format/html/format-html.ts:589:30)
        at async Object.formatExtras (file:///D:/a/quarto-cli/quarto-cli/src/format/html/format-html.ts:207:11)
        at async runPandoc (file:///D:/a/quarto-cli/quarto-cli/src/command/render/pandoc.ts:526:10)
        at async renderPandoc (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render.ts:202:24)
        at async Object.onRender (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:720:30)
        at async renderFileInternal (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:682:9)
        at async renderFiles (file:///D:/a/quarto-cli/quarto-cli/src/command/render/render-files.ts:330:9)
    throw new AssertionError(msg);
          ^
    at assert (https://jsr.io/@std/assert/0.224.0/assert.ts:18:11)
    at Module.fail (https://jsr.io/@std/assert/0.224.0/fail.ts:17:3)
    at fail (https://jsr.io/@std/testing/0.224.0/asserts.ts:663:11)
    at fn (file:///D:/a/quarto-cli/quarto-cli/tests/test.ts:303:11)

./smoke/smoke-all.test.ts (uncaught error)
error: (in promise) TypeError: error sending request for url (https://giscus.app/api/discussions/categories?repo=allenmanning/giscus-examples): client error (Connect): tcp connect error: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060): A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond. (os error 10060)
  const response = await fetch(url);
                   ^
    at async mainFetch (ext:deno_fetch/26_fetch.js:170:12)
    at async fetch (ext:deno_fetch/26_fetch.js:392:7)
    at async getGithubDiscussionsMetadata (file:///D:/a/quarto-cli/quarto-cli/src/core/giscus.ts:38:20)
    at async resolveFormatForGiscus (file:///D:/a/quarto-cli/quarto-cli/src/project/types/website/website-giscus.ts:65:24)
This error was not caught from a test and caused the test runner to fail on the referenced module.
It most likely originated from a dangling promise, event/timeout handler or top-level code.

Oddly it was only on windows: https://github.com/quarto-dev/quarto-cli/actions/runs/17297423070/job/49174379603

It seems we call metadata for giscus from the repo in
https://github.com/quarto-dev/quarto-cli/blob/main/src/project/types/website/website-giscus.ts

SO the smoke-all test may just test that this does not fail 🤷‍♂️

[cderv]

While doing the test with

website:
  comments:
    giscus:
      repo: allenmanning/giscus-examples

we do get to

quarto-cli/src/project/types/website/website-giscus.ts

Lines 70 to 76 in 4bba7f4

	} catch {
	// Couldn't read the cached metadata, fetch it
	removeIfExists(path);
	giscusMeta = await getGithubDiscussionsMetadata(repo);
	const jsonStr = JSON.stringify(giscusMeta, undefined, 2);
	Deno.writeTextFileSync(path, jsonStr);
	}

but this fetch leads to error we don't even catch

quarto-cli/src/project/types/website/website-giscus.ts

Line 73 in 4bba7f4

giscusMeta = await getGithubDiscussionsMetadata(repo);

> giscusMeta
{
  error: "giscus is not installed on this repository",
}

We don't handle non configured repo at this stage

quarto-cli/src/core/giscus.ts

Lines 32 to 41 in 4bba7f4

	export async function getGithubDiscussionsMetadata(repo: string) {
	const url = encodeURI(
	`https://giscus.app/api/discussions/categories?repo=${repo}`,
	);
	// Fetch repo info
	const response = await fetch(url);
	const jsonObj = await response.json();
	return jsonObj as GithubDiscussionMetadata;
	}

we probably should... 🤔

[cderv]

Maybe we could do

diff --git a/src/core/giscus.ts b/src/core/giscus.ts
index add751582..1113f7e7e 100644
--- a/src/core/giscus.ts
+++ b/src/core/giscus.ts
@ -36,6 +35,11 @@ export async function getGithubDiscussionsMetadata(repo: string) {

   // Fetch repo info
   const response = await fetch(url);
+  if (!response.ok) {
+    throw new Error(
+      `Failed to fetch discussions metadata: ${response.statusText} from ${repo}. Check giscus repo setting, and check this repo is correctly configured with giscus.`,
+    );
+  }
   const jsonObj = await response.json();
   return jsonObj as GithubDiscussionMetadata;
 }

And then test an unconfigured repo to check it errors, and a configured repo to check it does not error and get the right content.

Initially, the smoke-all tests were added for

• When using a dark theme, giscus should use a dark theme as well. #4820

And maybe we do test giscus light / dark already, and better using playwright. I remember something like this...

[gordonwoodhull]

I see... I didn't realize there was a render-time fetch for giscus. That makes much more sense!

And it was only failing for Windows, which explains why I was unable to repro locally.

Okay, yeah this "fix" is inadequate and I agree we need better error handling to get consistent results.

Now that I think of it, I'm actually not sure if quarto-dev/quarto-cli is valid... quarto-dev/quarto-web is the one where we have giscus enabled.

Okay, I'll close this for now. Too much of a rabbit hole and I have slides to prepare. 😄