🐛 Drop ReleaseNotes singleton to fix multi-pass state leak (#111)

lib/analyze-commits.js

@@ -8,7 +8,7 @@ module.exports = function analyzeCommits (pluginConfig, context) {
   const config = getConfig(pluginConfig, context)
   debug('[Analyze commits] Effective config: %o', config)
 
-  const notes = ReleaseNotes.get(context, config.releaseNotes)
+  const notes = new ReleaseNotes(context, config.releaseNotes)
 
   const releaseType = notes.getReleaseType(config.releaseRules)
   if (releaseType) {

lib/generate-notes.js

@@ -8,7 +8,7 @@ module.exports = async function generateNotes (pluginConfig, context) {
   const config = getConfig(pluginConfig, context)
   debug('[Generate notes] Effective config: %o', config)
 
-  const notes = ReleaseNotes.get(context, config.releaseNotes)
+  const notes = new ReleaseNotes(context, config.releaseNotes)
   await notes.updateContext(context)
 
   logger.log('Release notes are generated.')

lib/release-notes.js

@@ -161,12 +161,4 @@ module.exports = class ReleaseNotes {
         .replace(/\n{3,}/g, '\n\n') // allow a blank line
     }
   }
-
-  /**
-   * @return {ReleaseNotes}
-   */
-  static get (context, releaseNotesOptions) {
-    ReleaseNotes._instance = ReleaseNotes._instance || new ReleaseNotes(context, releaseNotesOptions)
-    return ReleaseNotes._instance
-  }
 }

test/integration/analyze-commits.test.js

@@ -1,20 +1,8 @@
 const test = require('ava')
-const sinon = require('sinon')
 
 const { analyzeCommits } = require('../..')
-const ReleaseNotes = require('../../lib/release-notes')
 const getContext = require('./fixtures/contexts')
 
-const stub = sinon.stub(ReleaseNotes, 'get')
-// to avoid singleton for tests
-stub.callsFake(function (...args) {
-  return new ReleaseNotes(...args)
-})
-
-test.after(function () {
-  stub.restore()
-})
-
 const CASES = [
   {
     name: 'default config + common context w/o updates',

test/integration/generate-notes.test.js

@@ -1,9 +1,7 @@
 const test = require('ava')
-const sinon = require('sinon')
 const dateFormat = require('dateformat')
 
 const { generateNotes } = require('../..')
-const ReleaseNotes = require('../../lib/release-notes')
 const getContext = require('./fixtures/contexts')
 
 const { readFileSync } = require('fs')
@@ -18,16 +16,6 @@ function readNotesSync (name) {
     .replace(/\{datetime\}/g, dateFormat(now, 'UTC:yyyy-mm-dd'))
 }
 
-const stub = sinon.stub(ReleaseNotes, 'get')
-// to avoid singleton for tests
-stub.callsFake(function (...args) {
-  return new ReleaseNotes(...args)
-})
-
-test.after(function () {
-  stub.restore()
-})
-
 const CASES = [
   {
     name: 'default config + common context w/ patch updates',

test/integration/release-notes-singleton.test.js

@@ -0,0 +1,54 @@
+const test = require('ava')
+
+const { analyzeCommits, generateNotes } = require('../..')
+const getContext = require('./fixtures/contexts')
+
+// Regression: semantic-release runs analyzeCommits / generateNotes twice
+// within the same process when adding a channel to an existing tag (e.g.
+// maintenance branch creation). State must not leak between passes.
+
+test.serial(
+  'analyzeCommits: pass 2 must not inherit release type cached from pass 1',
+  async function (t) {
+    // GIVEN pass 1 yields a "minor" release (`:sparkles:` commit present)
+    const pass1 = getContext('common', { commits: { boring: 1, minor: 1 } })
+    pass1.logger.log = t.log
+    t.is(await analyzeCommits({}, pass1), 'minor')
+
+    // WHEN pass 2 runs with only commits that match no release rule
+    const pass2 = getContext('common', { commits: { boring: 2 } })
+    pass2.logger.log = t.log
+
+    // THEN pass 2 must yield no release (would inherit 'minor' under the
+    // previous singleton + `_rtype` cache).
+    t.is(await analyzeCommits({}, pass2), undefined)
+  }
+)
+
+test.serial(
+  'generateNotes: pass 2 must render only commits from its own context',
+  async function (t) {
+    // GIVEN pass 1 rendered notes for a minor release including `:sparkles:` commits
+    const pass1 = getContext('common', {
+      commits: { boring: 1, minor: 1 },
+      nextRelease: { version: '1.1.0', gitTag: 'v1.1.0' }
+    })
+    pass1.logger.log = t.log
+    await analyzeCommits({}, pass1)
+    const pass1Notes = await generateNotes({}, pass1)
+    t.true(pass1Notes.includes('Add a new feature'))
+
+    // WHEN pass 2 generates notes for a context with no `:sparkles:` commit
+    const pass2 = getContext('common', {
+      commits: { boring: 1 },
+      nextRelease: { version: '1.1.1', gitTag: 'v1.1.1' }
+    })
+    pass2.logger.log = t.log
+
+    // THEN pass 2 notes must not include the `:sparkles:` commit from pass 1
+    // (would leak under the previous singleton whose `_context.commits` was
+    // frozen at first construction).
+    const pass2Notes = await generateNotes({}, pass2)
+    t.false(pass2Notes.includes('Add a new feature'))
+  }
+)