Repository: netlify/build
Pull request #1457 (2020-06-05 23:18:49)

Refactor how `utils.cache` is tested
`utils.cache` guesses which cache directory is the most appropriate depending on the environment. In order to test this behavior, we previously relied on an environment variable `NETLIFY_BUILD_TEST`.

Environment variables are global, so they make it harder to run several tests in parallel inside the same process. This PR switches to testing for the presence of the parameter/flag `testOpts.cachePath` (also added by #1430) instead.

It also updates some tests accordingly.


Patch:
diff --git a/packages/build/src/core/config.js b/packages/build/src/core/config.js
index 9b80f744e..dd6386d43 100644
--- a/packages/build/src/core/config.js
+++ b/packages/build/src/core/config.js
@@ -85,7 +85,7 @@ const loadConfig = async function(
   logContext(contextA)
 
   const apiA = addApiErrorHandlers(api)
-  const constants = await getConstants({ configPath, buildDir, netlifyConfig, siteInfo, deployId, mode })
+  const constants = await getConstants({ configPath, buildDir, netlifyConfig, siteInfo, deployId, mode, testOpts })
   return {
     netlifyConfig,
     configPath,
diff --git a/packages/build/src/core/constants.js b/packages/build/src/core/constants.js
index fa128c6ec..7ca40fd5f 100644
--- a/packages/build/src/core/constants.js
+++ b/packages/build/src/core/constants.js
@@ -16,10 +16,11 @@ const getConstants = async function({
   siteInfo: { id: siteId },
   deployId,
   mode,
+  testOpts: { cachePath: testCachePath },
 }) {
   const isLocal = mode !== 'buildbot'
   const functionsDist = getFunctionsDist(isLocal, deployId)
-  const cacheDir = await getCacheDir({ mode })
+  const cacheDir = await getCacheDir({ mode, isTest: testCachePath !== undefined })
 
   const constants = {
     /**
diff --git a/packages/cache-utils/src/dir.js b/packages/cache-utils/src/dir.js
index a6e565beb..170dab60a 100644
--- a/packages/cache-utils/src/dir.js
+++ b/packages/cache-utils/src/dir.js
@@ -1,10 +1,10 @@
 const { resolve } = require('path')
-const { platform, env } = require('process')
+const { platform } = require('process')
 
 const globalCacheDir = require('global-cache-dir')
 
 // Retrieve the cache directory location
-const getCacheDir = function({ cacheDir, mode } = {}) {
+const getCacheDir = function({ cacheDir, mode, isTest } = {}) {
   if (cacheDir !== undefined) {
     return resolve(cacheDir)
   }
@@ -14,7 +14,7 @@ const getCacheDir = function({ cacheDir, mode } = {}) {
   }
 
   // Do not use in tests since /opt might not be writable by current user
-  if (platform === 'linux' && !env.NETLIFY_BUILD_TEST) {
+  if (platform === 'linux' && !isTest) {
     return CI_CACHE_DIR
   }
 


Test patch:
diff --git a/packages/cache-utils/tests/dir.js b/packages/cache-utils/tests/dir.js
index 265a1d1d0..bc8c027dc 100644
--- a/packages/cache-utils/tests/dir.js
+++ b/packages/cache-utils/tests/dir.js
@@ -3,10 +3,9 @@ const pathExists = require('path-exists')
 
 const cacheUtils = require('..')
 
-const { pWriteFile, createTmpDir, createTmpFile, removeFiles } = require('./helpers/main')
+const { pWriteFile, createTmpDir, removeFiles } = require('./helpers/main')
 
-// Need to be serial since we change the current directory
-test.serial('Should allow not changing the cache directory', async t => {
+test('Should allow not changing the cache directory', async t => {
   const [cacheDir, srcDir] = await Promise.all([createTmpDir(), createTmpDir()])
   const currentDir = process.cwd()
   process.chdir(srcDir)
@@ -23,17 +22,7 @@ test.serial('Should allow not changing the cache directory', async t => {
   }
 })
 
-// Need to be serial since we change the environment variables
-test.serial('Should allow not changing the cache directory in CI', async t => {
-  const [cacheDir, [srcFile, srcDir]] = await Promise.all([createTmpDir(), createTmpFile()])
-  process.env.NETLIFY_BUILD_TEST = '1'
-  try {
-    t.true(await cacheUtils.save(srcFile, { mode: 'buildbot' }))
-    await removeFiles(srcFile)
-    t.true(await cacheUtils.restore(srcFile, { mode: 'buildbot' }))
-    t.true(await pathExists(srcFile))
-  } finally {
-    delete process.env.NETLIFY_BUILD_TEST
-    await removeFiles([cacheDir, srcDir])
-  }
+test('Should allow not changing the cache directory in CI', async t => {
+  const cacheDir = await cacheUtils.getCacheDir({ mode: 'buildbot', isTest: true })
+  t.true(await pathExists(cacheDir))
 })