From dcd5823a17f21a932d36317f25dd3b91d37710c5 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Mon, 23 Sep 2019 11:47:53 +0200 Subject: [PATCH 1/6] Fix concurrent tests. Running jest runs the two test suites in parallel. If both try to run a server on port 3000, only one of them will succeed and supply files for both, however when that suite finishes first, it will take down the server and leave the other one hanging. Fix this by running the servers on different ports. --- integration/widget.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/widget.test.js b/integration/widget.test.js index dd838ac..2362b9a 100644 --- a/integration/widget.test.js +++ b/integration/widget.test.js @@ -3,14 +3,14 @@ const express = require('express'); const puppeteer = require('puppeteer') -const rootUrl = "http://localhost:3000/"; +const rootUrl = "http://localhost:3001/"; jest.setTimeout(500000); const server = new Promise( resolve => { let app = require( '../src/server' )({ directory: path.join( __dirname, '..' ) }); let server; - server = app.listen( 3000, () => resolve(server) ); + server = app.listen( 3001, () => resolve(server) ); }).catch( e => console.log(e) ); const browser = puppeteer.launch({ headless: false }); From 07e4c13530099658866ae069092aa79014567e33 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Tue, 24 Sep 2019 14:50:17 +0200 Subject: [PATCH 2/6] Make tests faster by avoiding unnecessary waiting. Puppeteer allows us to wait for the specific element we want to check instead of waiting for a fixed time and hoping it will be there by then. This reduces the total testing time from 26s to 7s for me. --- integration/basic.test.js | 8 ++++---- integration/widget.test.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/integration/basic.test.js b/integration/basic.test.js index 11b01de..2f6a8e8 100644 --- a/integration/basic.test.js +++ b/integration/basic.test.js @@ -40,7 +40,7 @@ test('resolve #definitions in non-root schema', async () => { await page.goto( rootUrl + "#/integration/schemas/def-non-root/User.json"); - await page.waitFor(5000); + await page.waitFor('#doc .box .box .box .signature:nth-child(6) .property-name'); await expect( page.evaluate( () => Array.from(document.querySelectorAll('.property-name').values()).map( s => s.innerText ) ) @@ -55,7 +55,7 @@ test('local schema, absolute path', async () => { await page.goto( rootUrl + "#/integration/schemas/local-absolute/main.json"); - await page.waitFor(5000); + await page.waitFor('#doc .box .box .desc'); await expect( page.evaluate( () => Array.from(document.querySelectorAll('.desc').values()).map( s => s.innerText ) ) @@ -70,7 +70,7 @@ test('recursive schemas', async () => { await page.goto( rootUrl + "#/integration/schemas/recursive/circle.json"); - await page.waitFor(5000); + await page.waitFor('#doc .box .box .box .desc'); await expect( page.evaluate( () => Array.from(document.querySelectorAll('.desc').values()).map( s => s.innerText ) ) @@ -84,7 +84,7 @@ test('recursive schemas, part II', async () => { const page = await ( await browser ).newPage(); await page.goto( rootUrl + "#/integration/schemas/recursive/within_schema.json"); - await page.waitFor(5000); + await page.waitFor('#doc .box .desc p'); let results = await page.evaluate( () => Array.from(document.querySelectorAll('p').values()).map( s => s.innerText ) ); diff --git a/integration/widget.test.js b/integration/widget.test.js index 2362b9a..7aea0bf 100644 --- a/integration/widget.test.js +++ b/integration/widget.test.js @@ -31,7 +31,7 @@ test( 'basic', async () => { let frames = await page.frames(); - await page.waitFor(2000); + await frames[1].waitFor('.title'); let title = await frames[1].evaluate( () => document.querySelector('.title').innerText From 396c9f0c8532068eb3507a8692c3f7d540b9fa4a Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Thu, 26 Sep 2019 16:02:02 +0200 Subject: [PATCH 3/6] Fix "ReferenceError: Can't find variable: segments" masking JSON parsing errors. This appears to have been a copy-paste error in 6b97aa2 and caused rendering of schemas that triggered such parsing errors to be aborted before anything was rendered. Also output the content that failed to parse for more info. This brings test.html output back to what is currently seen at http://lbovet.github.io/docson/tests/test.html, although with a wrongful such JSON parsing error on the console, to be fixed in the next commit. --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 5e40ba4..ba979f1 100644 --- a/src/index.js +++ b/src/index.js @@ -53,7 +53,7 @@ const highlight = false; try { content = JSON.parse(content); } catch(e) { - console.error("Unable to parse "+segments[0], e); + console.error("Unable to parse "+url, e, content); content = {}; } } From e62b914687922565b1a80c1569390a703242ec09 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Thu, 26 Sep 2019 16:04:20 +0200 Subject: [PATCH 4/6] Fix attempt to parse the HTML page as JSON for the root schema. In schemas that contained internal references ({"$ref": "#..."}), Docson would attempt to retrieve the document named by that URI using $.get(). That would result in the HTML code for the page it was running in, which is completely unrelated and would fail to parse as JSON, resulting in an error message on the console. To avoid attempting to retrieve the root document again, initialize the document collection with it already. As a side effect, this causes a circular reference to the root schema to be detected at the first cycle rather than the second, which makes integration test "recursive schema" fail, but actually seems desirable to me. --- src/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/index.js b/src/index.js index ba979f1..d4f84a7 100644 --- a/src/index.js +++ b/src/index.js @@ -572,6 +572,7 @@ const highlight = false; return Promise.all(p).finally(); }; + schemaDocuments[baseUrl] = Promise.resolve(schema); resolveRefsReentrant(schema).finally( throttled_render ).then( resolve_d ).catch( e => console.log('oops', e ) ); }) return d.finally( throttled_render ); From d5ae761b7f9ab78cb39ec08b690951a9e81c0fac Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Thu, 26 Sep 2019 17:38:50 +0200 Subject: [PATCH 5/6] Fix circular reference test. The previous commit causes a circular reference to the root schema to be detected at the first cycle rather than the second, like anywhere else. That actually seems to make more sense to me, so change the test that previously checked for detection at the second cycle. --- integration/basic.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/basic.test.js b/integration/basic.test.js index 2f6a8e8..14ee919 100644 --- a/integration/basic.test.js +++ b/integration/basic.test.js @@ -70,7 +70,7 @@ test('recursive schemas', async () => { await page.goto( rootUrl + "#/integration/schemas/recursive/circle.json"); - await page.waitFor('#doc .box .box .box .desc'); + await page.waitFor('#doc .box .box .desc'); await expect( page.evaluate( () => Array.from(document.querySelectorAll('.desc').values()).map( s => s.innerText ) ) From 36e874540f99c03b46854dc508238813747a7f4e Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Thu, 26 Sep 2019 16:07:00 +0200 Subject: [PATCH 6/6] Fix {"$ref": "#"} not appearing in the output. It was mistakenly looked up under URI "" instead of "#". --- src/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index d4f84a7..7425060 100644 --- a/src/index.js +++ b/src/index.js @@ -540,8 +540,10 @@ const highlight = false; uri.normalize(); - // use the normalized uri - parentObject.update( uri.toString() ); + // use the normalized uri, unless it's empty (which happens when item == "#" and baseUrl is empty) + if (uri.toString()) { + parentObject.update( uri.toString() ); + } debug(get_ref(uri)); get_ref( uri ).finally( () => {