From 4a2590fa78d5c73f19d1e1dc0779a2437874c7b3 Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Fri, 15 Apr 2022 11:20:59 -0400
Subject: [PATCH] re-enable some skipped tests  (#4614)

* re-enable some skipped tests (#1245)

* remove deprecated option

* what the hell
---
 .changeset/rude-tips-rule.md                   |  5 +++++
 .../sync/create_manifest_data/index.spec.js    |  2 +-
 packages/kit/src/runtime/server/endpoint.js    | 18 ++++++++++++++++--
 packages/kit/test/apps/basics/test/test.js     | 12 ++++++------
 4 files changed, 28 insertions(+), 9 deletions(-)
 create mode 100644 .changeset/rude-tips-rule.md

diff --git a/.changeset/rude-tips-rule.md b/.changeset/rude-tips-rule.md
new file mode 100644
index 000000000000..4fbc23888a14
--- /dev/null
+++ b/.changeset/rude-tips-rule.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Include disallowed method name in 405 response, include Allow header
diff --git a/packages/kit/src/core/sync/create_manifest_data/index.spec.js b/packages/kit/src/core/sync/create_manifest_data/index.spec.js
index 2c0bfdc6c44f..76a2adab2934 100644
--- a/packages/kit/src/core/sync/create_manifest_data/index.spec.js
+++ b/packages/kit/src/core/sync/create_manifest_data/index.spec.js
@@ -129,7 +129,7 @@ test('creates routes with layout', () => {
 });
 
 // TODO some characters will need to be URL-encoded in the filename
-test.skip('encodes invalid characters', () => {
+test('encodes invalid characters', () => {
 	const { components, routes } = create('samples/encoding');
 
 	// had to remove ? and " because windows
diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js
index 433d920612ad..d29003cbc4de 100644
--- a/packages/kit/src/runtime/server/endpoint.js
+++ b/packages/kit/src/runtime/server/endpoint.js
@@ -50,14 +50,28 @@ export async function render_endpoint(event, mod) {
 	}
 
 	if (!handler) {
+		const allowed = [];
+
+		for (const method in ['get', 'post', 'put', 'patch']) {
+			if (mod[method]) allowed.push(method.toUpperCase());
+		}
+
+		if (mod.del) allowed.push('DELETE');
+		if (mod.get || mod.head) allowed.push('HEAD');
+
 		return event.request.headers.get('x-sveltekit-load')
 			? // TODO would be nice to avoid these requests altogether,
 			  // by noting whether or not page endpoints export `get`
 			  new Response(undefined, {
 					status: 204
 			  })
-			: new Response('Method not allowed', {
-					status: 405
+			: new Response(`${event.request.method} method not allowed`, {
+					status: 405,
+					headers: {
+						// https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405
+						// "The server must generate an Allow header field in a 405 status code response"
+						allow: allowed.join(', ')
+					}
 			  });
 	}
 
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index 2e7dd06b5955..ff7162177670 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -876,11 +876,11 @@ test.describe.parallel('Errors', () => {
 	// TODO before we implemented route fallthroughs, and there was a 1:1
 	// regex:route relationship, it was simple to say 'method not implemented
 	// for this endpoint'. now it's a little tricker. does a 404 suffice?
-	test.skip('unhandled http method', async ({ request }) => {
+	test('unhandled http method', async ({ request }) => {
 		const response = await request.put('/errors/invalid-route-response');
 
-		expect(/** @type {import('@playwright/test').APIResponse} */ (response).status()).toBe(501);
-		expect(await response.text()).toMatch('PUT is not implemented');
+		expect(response.status()).toBe(405);
+		expect(await response.text()).toMatch('PUT method not allowed');
 	});
 
 	test('error in endpoint', async ({ page, read_errors }) => {
@@ -2078,8 +2078,7 @@ test.describe.parallel('Routing', () => {
 		server.close();
 	});
 
-	// skipping this test because it causes a bunch of failures locally
-	test.skip('watch new route in dev', async ({ page, javaScriptEnabled }) => {
+	test('watch new route in dev', async ({ page, javaScriptEnabled }) => {
 		await page.goto('/routing');
 
 		if (!process.env.DEV || javaScriptEnabled) {
@@ -2091,10 +2090,11 @@ test.describe.parallel('Routing', () => {
 		const route = 'bar' + new Date().valueOf();
 		const content = 'Hello new route';
 		const __dirname = path.dirname(fileURLToPath(import.meta.url));
-		const filePath = path.join(__dirname, `${route}.svelte`);
+		const filePath = path.join(__dirname, `../src/routes/routing/${route}.svelte`);
 
 		try {
 			fs.writeFileSync(filePath, `<h1>${content}</h1>`);
+			await page.waitForTimeout(250); // this is the rare time we actually need waitForTimeout; we have no visibility into whether the module graph has been invalidated
 			await page.goto(`/routing/${route}`);
 
 			expect(await page.textContent('h1')).toBe(content);