Skip to content

Comments

add exception handle to include#54

Open
Industry-Standard wants to merge 5 commits intoarnaudsm:masterfrom
Industry-Standard:add-404-for-bad-include
Open

add exception handle to include#54
Industry-Standard wants to merge 5 commits intoarnaudsm:masterfrom
Industry-Standard:add-404-for-bad-include

Conversation

@Industry-Standard
Copy link

Addresses issue #47, basic try catch on the include marked extension.

@Industry-Standard
Copy link
Author

Actually, let me remove the comments and extra function - I'll resubmit.

@Industry-Standard
Copy link
Author

Added a new test for bad includes, all tests passed:

yarn run v1.22.22
$ npx playwright test

Running 36 tests using 3 workers

  ✓  1 [hash] › main.test.js:20:1 › Homepage (337ms)
  ✓  2 [star] › main.test.js:20:1 › Homepage (333ms)
  ✓  3 [subdir] › main.test.js:20:1 › Homepage (332ms)
  ✓  4 [subdir] › main.test.js:25:1 › Docs (326ms)
  ✓  5 [hash] › main.test.js:25:1 › Docs (292ms)
  ✓  6 [star] › main.test.js:25:1 › Docs (317ms)
  ✓  7 [hash] › main.test.js:30:1 › Navbar (344ms)
  ✓  8 [subdir] › main.test.js:30:1 › Navbar (334ms)
  ✓  9 [star] › main.test.js:30:1 › Navbar (335ms)
  ✓  10 [hash] › main.test.js:39:1 › Anchors (355ms)
  ✓  11 [subdir] › main.test.js:39:1 › Anchors (377ms)
  ✓  12 [star] › main.test.js:39:1 › Anchors (373ms)
  ✓  13 [hash] › main.test.js:46:1 › Include (296ms)
  ✓  14 [star] › main.test.js:46:1 › Include (267ms)
  ✓  15 [subdir] › main.test.js:46:1 › Include (282ms)
  ✓  16 [hash] › main.test.js:51:1 › Bad Include (282ms)
  ✓  17 [star] › main.test.js:51:1 › Bad Include (284ms)
  ✓  18 [subdir] › main.test.js:51:1 › Bad Include (264ms)
  ✓  19 [hash] › main.test.js:56:1 › History (398ms)
  ✓  20 [subdir] › main.test.js:56:1 › History (443ms)
  ✓  21 [star] › main.test.js:56:1 › History (428ms)
  ✓  22 [hash] › main.test.js:70:1 › Subdirectories (264ms)
  ✓  23 [star] › main.test.js:70:1 › Subdirectories (285ms)
  ✓  24 [subdir] › main.test.js:70:1 › Subdirectories (278ms)
  ✓  25 [hash] › main.test.js:88:1 › Root (219ms)
  ✓  26 [star] › main.test.js:88:1 › Root (234ms)
  ✓  27 [subdir] › main.test.js:88:1 › Root (300ms)
  ✓  28 [hash] › main.test.js:96:1 › Internal Anchors (544ms)
  ✓  29 [star] › main.test.js:96:1 › Internal Anchors (247ms)
  ✓  30 [subdir] › main.test.js:96:1 › Internal Anchors (243ms)
  ✓  31 [star] › main.test.js:103:1 › Internal Anchors Direct (283ms)
  ✓  32 [subdir] › main.test.js:103:1 › Internal Anchors Direct (283ms)
  ✓  33 [hash] › main.test.js:103:1 › Internal Anchors Direct (297ms)
  ✓  34 [star] › main.test.js:109:1 › 404 (302ms)
  ✓  35 [subdir] › main.test.js:109:1 › 404 (280ms)
  ✓  36 [hash] › main.test.js:109:1 › 404 (289ms)

  36 passed (4.4s)

@dude-at-RA
Copy link

@arnaudsm - what else needs to be done to merge this?

Copy link
Owner

@arnaudsm arnaudsm left a comment

Choose a reason for hiding this comment

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

My apologies, I was quite busy these past few months.

Thank you very much for your contribution, this is a great addition !

Are you able to merge this ? Testing the github permissions

request-checks: true
@Industry-Standard
Copy link
Author

@arnaudsm I made a new commit with request-checks: true in the message body, but I'm not sure what's required at this point to merge. I think you may need to approve the pending check?

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.

3 participants