(core) updates from grist-core

test/server/lib/AppSettings.ts

@@ -0,0 +1,96 @@
+import { AppSettings } from 'app/server/lib/AppSettings';
+import { EnvironmentSnapshot } from '../testUtils';
+
+import { assert } from 'chai';
+
+describe('AppSettings', () => {
+  let appSettings: AppSettings;
+  let env: EnvironmentSnapshot;
+  beforeEach(() => {
+    appSettings = new AppSettings('test');
+    env = new EnvironmentSnapshot();
+  });
+
+  afterEach(() => {
+    env.restore();
+  });
+
+  describe('for integers', () => {
+    function testIntMethod(method: 'readInt' | 'requireInt') {
+      it('should throw an error if the value is less than the minimum', () => {
+        process.env.TEST = '4';
+        assert.throws(() => {
+          appSettings[method]({ envVar: 'TEST', minValue: 5 });
+        }, 'value 4 is less than minimum 5');
+      });
+
+      it('should throw an error if the value is greater than the maximum', () => {
+        process.env.TEST = '6';
+        assert.throws(() => {
+          appSettings[method]({ envVar: 'TEST', maxValue: 5 });
+        }, 'value 6 is greater than maximum 5');
+      });
+
+      it('should throw if the value is NaN', () => {
+        process.env.TEST = 'not a number';
+        assert.throws(() => appSettings[method]({ envVar: 'TEST' }), 'not a number does not look like a number');
+      });
+
+      it('should throw if the default value is not finite', () => {
+        assert.throws(
+          () => appSettings[method]({ envVar: 'TEST', defaultValue: Infinity }),
+          'Infinity does not look like a number'
+        );
+      });
+
+      it('should throw if the default value is not within the range', () => {
+        assert.throws(
+          () => appSettings[method]({
+            envVar: 'TEST',
+            defaultValue: 6,
+            minValue: 7,
+            maxValue: 9,
+          }),
+          'value 6 is less than minimum 7'
+        );
+      });
+
+      it('should return the default value if it is within the range', () => {
+        const result = appSettings[method]({
+          envVar: 'TEST',
+          defaultValue: 5,
+          minValue: 5,
+          maxValue: 12
+        });
+        assert.strictEqual(result, 5);
+      });
+
+      it('should return the value if it is within the range', () => {
+        process.env.TEST = '5';
+        assert.strictEqual(appSettings[method]({ envVar: 'TEST', minValue: 5 }), 5);
+      });
+
+      it('should return the integer value of a float', () => {
+        process.env.TEST = '5.9';
+        assert.strictEqual(appSettings[method]({ envVar: 'TEST' }), 5);
+      });
+    }
+
+    describe('readInt()', () => {
+      testIntMethod('readInt');
+
+      it('should return undefined when no value nor default value is passed', () => {
+        const result = appSettings.readInt({ envVar: 'TEST', maxValue: 5 });
+        assert.isUndefined(result);
+      });
+    });
+
+    describe('requireInt()', () => {
+      testIntMethod('requireInt');
+
+      it('should throw if env variable is not set and no default value is passed', () => {
+        assert.throws(() => appSettings.requireInt({ envVar: 'TEST' }), 'missing environment variable: TEST');
+      });
+    });
+  });
+});

test/server/lib/DocApi.ts

@@ -76,6 +76,8 @@ function makeConfig(username: string): AxiosRequestConfig {
 }
 
 describe('DocApi', function () {
+  const webhooksTestPort = Number(process.env.WEBHOOK_TEST_PORT || 34365);
+
   this.timeout(30000);
   testUtils.setTmpLogLevel('error');
   let oldEnv: testUtils.EnvironmentSnapshot;
@@ -121,7 +123,7 @@ describe('DocApi', function () {
       homeUrl = serverUrl = home.serverUrl;
       hasHomeApi = true;
     });
-    testDocApi();
+    testDocApi({webhooksTestPort});
   });
 
   describe('With GRIST_ANON_PLAYGROUND disabled', async () => {
@@ -157,7 +159,7 @@ describe('DocApi', function () {
         homeUrl = serverUrl = home.serverUrl;
         hasHomeApi = true;
       });
-      testDocApi();
+      testDocApi({webhooksTestPort});
     });
 
     describe('behind a reverse-proxy', function () {
@@ -206,7 +208,7 @@ describe('DocApi', function () {
 
         after(() => tearDown(proxy, [home, docs]));
 
-        testDocApi();
+        testDocApi({webhooksTestPort});
       });
 
       async function testCompareDocs(proxy: TestServerReverseProxy, home: TestServer) {
@@ -261,7 +263,7 @@ describe('DocApi', function () {
         serverUrl = docs.serverUrl;
         hasHomeApi = false;
       });
-      testDocApi();
+      testDocApi({webhooksTestPort});
     });
   }
 
@@ -323,7 +325,10 @@ describe('DocApi', function () {
 });
 
 // Contains the tests. This is where you want to add more test.
-function testDocApi() {
+function testDocApi(settings: {
+  webhooksTestPort: number,
+}) {
+  const { webhooksTestPort } = settings;
   let chimpy: AxiosRequestConfig, kiwi: AxiosRequestConfig,
     charon: AxiosRequestConfig, nobody: AxiosRequestConfig, support: AxiosRequestConfig;
 
@@ -3478,13 +3483,20 @@ function testDocApi() {
   });
 
   describe('webhooks related endpoints', async function () {
-    const serving: Serving = await serveSomething(app => {
-      app.use(express.json());
-      app.post('/200', ({body}, res) => {
-        res.sendStatus(200);
-        res.end();
-      });
-    }, webhooksTestPort);
+    let serving: Serving;
+    before(async function () {
+      serving = await serveSomething(app => {
+        app.use(express.json());
+        app.post('/200', ({body}, res) => {
+          res.sendStatus(200);
+          res.end();
+        });
+      }, webhooksTestPort);
+    });
+
+    after(async function () {
+      await serving.shutdown();
+    });
 
     /*
       Regression test for old _subscribe endpoint. /docs/{did}/webhooks should be used instead to subscribe
@@ -3577,7 +3589,8 @@ function testDocApi() {
           }
         }]
       },
-        403, /Column not found notExisting/);
+        // this check was previously just wrong, was the test not running somehow??
+        404, /Column not found "notExisting"/);
 
     });
 
@@ -5385,8 +5398,6 @@ async function getWorkspaceId(api: UserAPIImpl, name: string) {
   return workspaces.find((w) => w.name === name)!.id;
 }
 
-const webhooksTestPort = Number(process.env.WEBHOOK_TEST_PORT || 34365);
-
 async function setupDataDir(dir: string) {
   // we'll be serving Hello.grist content for various document ids, so let's make copies of it in
   // tmpDir

test/server/lib/ProxyAgent.ts

@@ -84,15 +84,17 @@ describe("ProxyAgent", function () {
     it("should report error when proxy fails", async function() {
       // if the proxy isn't listening, fetches produces error messages.
       await testProxyServer.dispose();
+      // Error message depends a little on node version.
       const logMessages2 = await captureLog('warn', async () => {
-        await assert.isRejected(testFetch('/200'), /ECONNREFUSED/);
-        await assert.isRejected(testFetch('/404'), /ECONNREFUSED/);
+        await assert.isRejected(testFetch('/200'), /(request.*failed)|(ECONNREFUSED)/);
+        await assert.isRejected(testFetch('/404'), /(request.*failed)|(ECONNREFUSED)/);
       });
 
       // We rely on "ProxyAgent error" message to detect issues with the proxy server.
+      // Error message depends a little on node version.
       assertMatchArray(logMessages2, [
-        /warn: ProxyAgent error.*ECONNREFUSED/,
-        /warn: ProxyAgent error.*ECONNREFUSED/,
+        /warn: ProxyAgent error.*((request.*failed)|(ECONNREFUSED)|(AggregateError))/,
+        /warn: ProxyAgent error.*((request.*failed)|(ECONNREFUSED)|(AggregateError))/,
       ]);
     });
   });

test/server/lib/UnhandledErrors.ts

@@ -51,7 +51,8 @@ describe('UnhandledErrors', function() {
         }, 1000, 100);
 
         // We expect the server to be dead now.
-        await assert.isRejected(fetch(`${server.serverUrl}/status`), /failed.*ECONNREFUSED/);
+        // Error message depends a little on node version.
+        await assert.isRejected(fetch(`${server.serverUrl}/status`), /(request.*failed)|(ECONNREFUSED)/);
 
       } finally {
         await server.stop();