From d92907358ec102130bd8f90a4d764355c53a0874 Mon Sep 17 00:00:00 2001
From: Kunal Kundu <51631122+tinfoil-knight@users.noreply.github.com>
Date: Fri, 24 Sep 2021 18:34:56 +0530
Subject: [PATCH] refactor: split core function of loadDotEnvFiles for better
 testing (#3396)

Co-authored-by: ehmicky <ehmicky@users.noreply.github.com>
---
 src/utils/dev.js          |  2 +-
 src/utils/dot-env.js      | 22 ++++++++++++++++++----
 src/utils/dot-env.test.js | 17 +++++++----------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/src/utils/dev.js b/src/utils/dev.js
index 354a716625d..b40e0893fda 100644
--- a/src/utils/dev.js
+++ b/src/utils/dev.js
@@ -131,7 +131,7 @@ const getEnvSourceName = (source) => {
 // dot-env files and the process itself, and injects into `process.env`.
 const injectEnvVariables = async ({ env, site }) => {
   const environment = new Map(Object.entries(env))
-  const dotEnvFiles = await loadDotEnvFiles({ projectDir: site.root, warn })
+  const dotEnvFiles = await loadDotEnvFiles({ projectDir: site.root })
 
   dotEnvFiles.forEach(({ file, env: fileEnv }) => {
     Object.keys(fileEnv).forEach((key) => {
diff --git a/src/utils/dot-env.js b/src/utils/dot-env.js
index 1022d4aa22b..38e90f3b3e6 100644
--- a/src/utils/dot-env.js
+++ b/src/utils/dot-env.js
@@ -4,7 +4,20 @@ const dotenv = require('dotenv')
 
 const { isFileAsync, readFileAsync } = require('../lib/fs')
 
-const loadDotEnvFiles = async function ({ projectDir, warn }) {
+const { warn } = require('./command-helpers')
+
+const loadDotEnvFiles = async function ({ projectDir }) {
+  const response = await tryLoadDotEnvFiles({ projectDir })
+
+  const filesWithWarning = response.filter((el) => el.warning)
+  filesWithWarning.forEach((el) => {
+    warn(el.warning)
+  })
+
+  return response.filter((el) => el.file && el.env)
+}
+
+const tryLoadDotEnvFiles = async ({ projectDir }) => {
   const dotenvFiles = ['.env', '.env.development']
   const results = await Promise.all(
     dotenvFiles.map(async (file) => {
@@ -15,8 +28,9 @@ const loadDotEnvFiles = async function ({ projectDir, warn }) {
           return
         }
       } catch (error) {
-        warn(`Failed reading env variables from file: ${filepath}: ${error.message}`)
-        return
+        return {
+          warning: `Failed reading env variables from file: ${filepath}: ${error.message}`,
+        }
       }
       const content = await readFileAsync(filepath)
       const env = dotenv.parse(content)
@@ -27,4 +41,4 @@ const loadDotEnvFiles = async function ({ projectDir, warn }) {
   return results.filter(Boolean)
 }
 
-module.exports = { loadDotEnvFiles }
+module.exports = { loadDotEnvFiles, tryLoadDotEnvFiles }
diff --git a/src/utils/dot-env.test.js b/src/utils/dot-env.test.js
index e18c8ae185c..61b44340a67 100644
--- a/src/utils/dot-env.test.js
+++ b/src/utils/dot-env.test.js
@@ -1,19 +1,16 @@
 const process = require('process')
 
 const test = require('ava')
-const sinon = require('sinon')
 
 const { withSiteBuilder } = require('../../tests/utils/site-builder')
 
-const { loadDotEnvFiles } = require('./dot-env')
+const { tryLoadDotEnvFiles } = require('./dot-env')
 
-const warn = sinon.stub()
-
-test('should return an object with empty array for a site with no .env file', async (t) => {
+test('should return an empty array for a site with no .env file', async (t) => {
   await withSiteBuilder('site-without-env-file', async (builder) => {
     await builder.buildAsync()
 
-    const results = await loadDotEnvFiles({ projectDir: builder.directory, warn })
+    const results = await tryLoadDotEnvFiles({ projectDir: builder.directory })
     t.deepEqual(results, [])
   })
 })
@@ -27,7 +24,7 @@ test('should read env vars from .env file', async (t) => {
     })
     await builder.buildAsync()
 
-    const results = await loadDotEnvFiles({ projectDir: builder.directory, warn })
+    const results = await tryLoadDotEnvFiles({ projectDir: builder.directory })
     t.deepEqual(results, [{ file: '.env', env: { TEST: 'FROM_ENV' } }])
   })
 })
@@ -41,7 +38,7 @@ test('should read env vars from .env.development file', async (t) => {
     })
     await builder.buildAsync()
 
-    const results = await loadDotEnvFiles({ projectDir: builder.directory, warn })
+    const results = await tryLoadDotEnvFiles({ projectDir: builder.directory })
     t.deepEqual(results, [{ file: '.env.development', env: { TEST: 'FROM_DEVELOPMENT_ENV' } }])
   })
 })
@@ -60,7 +57,7 @@ test('should read from both .env.development and .env', async (t) => {
       })
     await builder.buildAsync()
 
-    const results = await loadDotEnvFiles({ projectDir: builder.directory, warn })
+    const results = await tryLoadDotEnvFiles({ projectDir: builder.directory })
     t.deepEqual(results, [
       { file: '.env', env: { ONE: 'FROM_ENV', TWO: 'FROM_ENV' } },
       { file: '.env.development', env: { ONE: 'FROM_DEVELOPMENT_ENV', THREE: 'FROM_DEVELOPMENT_ENV' } },
@@ -76,7 +73,7 @@ test('should handle empty .env file', async (t) => {
 
     await builder.buildAsync()
 
-    const results = await loadDotEnvFiles({ projectDir: builder.directory, warn })
+    const results = await tryLoadDotEnvFiles({ projectDir: builder.directory })
     t.deepEqual(results, [{ file: '.env', env: {} }])
   })
 })