tests

build-filtered-data.mjs

@@ -120,10 +120,11 @@ export async function analyseRepo (repo) {
         ) {
           repoData.log('searching for version in lockfile')
           repoData.versionDoubt = true
-          // @TODO: Get lockfile type here and log it. Pass to next function
-          repoData.frontendVersion = await repoData.getVersionFromLockfile()
+          const lockfileType = repoData.getLockfileType()
+          repoData.log(`using ${lockfileType}`)
+          repoData.lockfileFrontendVersion = await repoData.getVersionFromLockfile()
           repoData.log(
-            `using GOV.UK Frontend version ${repoData.frontendVersion}`
+            `using GOV.UK Frontend version ${repoData.lockfileFrontendVersion}`
           )
         }
       }

helpers/repo-data.mjs

@@ -7,10 +7,10 @@ import {
 import * as yarnLock from '@yarnpkg/lockfile'
 import { RequestError } from 'octokit'
 
-class UnsupportedLockFileError extends Error {}
-class NoMetaDataError extends Error {}
-class NoRepoTreeError extends Error {}
-class NoCommitsError extends Error {}
+export class UnsupportedLockFileError extends Error {}
+export class NoMetaDataError extends Error {}
+export class NoRepoTreeError extends Error {}
+export class NoCommitsError extends Error {}
 
 export class RepoData {
   /**
@@ -32,7 +32,7 @@ export class RepoData {
     this.repoOwner = repoOwner
     this.repoName = repoName
     this.couldntAccess = false
-    this.frontendVersion = null
+    this.lockfileFrontendVersion = null
     this.versionDoubt = false
     this.builtByGovernment = serviceOwners.includes(this.repoOwner)
     this.indirectDependency = false
@@ -91,7 +91,7 @@ export class RepoData {
       this.repoName,
       latestCommitSha
     )
-    if (!this.repoTree) {
+    if (!this.repoTree || !this.repoTree.data || !this.repoTree.data.tree) {
       throw new NoRepoTreeError()
     }
   }
@@ -199,12 +199,11 @@ export class RepoData {
   }
 
   /**
-   * Gets the version of govuk-frontend from the lockfile
-   * @returns {string} - The version of govuk-frontend
-   * @throws {UnsupportedLockFileError} - If the lockfile is not supported
-   * @throws {RequestError} - If the request for the file data fails
+   * Checks for the type of lockfile
+   *
+   * @returns {string} - the lockfile type
    */
-  async getVersionFromLockfile () {
+  getLockfileType () {
     let lockfileType
     if (this.checkFileExists('package-lock.json')) {
       lockfileType = 'package-lock.json'
@@ -214,27 +213,36 @@ export class RepoData {
       // @TODO: support some package files - ruby (for GOV.UK) and maybe python?
       throw new UnsupportedLockFileError()
     }
+    return lockfileType
+  }
 
+  /**
+   * Gets the version of govuk-frontend from the lockfile
+   * @returns {string} - The version of govuk-frontend
+   * @throws {UnsupportedLockFileError} - If the lockfile is not supported
+   * @throws {RequestError} - If the request for the file data fails
+   */
+  async getVersionFromLockfile (lockfileType) {
     const lockfile = await this.getRepoFileContent(lockfileType)
 
     if (lockfileType === 'package-lock.json') {
       const lockfileObject = JSON.parse(lockfile.data)
-      if (this.frontendVersion) {
+      if (this.frontendVersions.length > 0) {
         // If we found an ambiguous frontend version in the package.json file,
         // all we have to do is get the package version from the lockfile
         const packageVersion =
           lockfileObject.packages?.['node_modules/govuk-frontend']?.version ||
           lockfileObject.dependencies?.['node_modules/govuk-frontend']?.version
         if (packageVersion) {
-          this.frontendVersion = packageVersion
+          this.lockfileFrontendVersion = packageVersion
         }
       } else {
         const deps = []
         // If we didn't find a frontend version in the package.json file,
         // we have to search the lockfile for the govuk-frontend entries
         for (const [packageName, packageData] of Object.entries({
           ...(lockfileObject.packages || {}),
-          ...(lockfileObject.dependencies || {}),
+          ...(lockfileObject.dependencies || {})
         })) {
           if (packageData.dependencies?.['govuk-frontend']) {
             deps.push({
@@ -244,14 +252,25 @@ export class RepoData {
           }
         }
         this.parentDependency = deps
+        // Set highest dependency number to frontendVersion.
+        // Not sure this is the right approach, but we'll still have dependency data
+        if (deps.length > 0) {
+          const versions = deps.map(dep => dep.version)
+          this.lockfileFrontendVersion = versions.sort((a, b) => b.localeCompare(a, undefined, { numeric: true }))[0]
+        }
       }
     } else if (lockfileType === 'yarn.lock') {
-      const yarnLockObject = yarnLock.default.parse(lockfile.data)
-      this.frontendVersion =
-        yarnLockObject.object[`govuk-frontend@${this.frontendVersion}`]
-          ?.version || this.frontendVersion
+      const yarnLockObject = yarnLock.parse(lockfile.data)
+      let yarnLockVersion = '0'
+
+      for (const [key, value] of Object.entries(yarnLockObject.object)) {
+        if (key.startsWith('govuk-frontend@') && value.version > yarnLockVersion) {
+          yarnLockVersion = value.version
+        }
+      }
+      this.lockfileFrontendVersion = yarnLockVersion || this.lockfileFrontendVersion
     }
-    return this.frontendVersion
+    return this.lockfileFrontendVersion
   }
 
   /**
@@ -290,7 +309,7 @@ export class RepoData {
       repoOwner: this.repoOwner,
       repoName: this.repoName,
       couldntAccess: this.couldntAccess,
-      frontendVersion: this.frontendVersion,
+      lockfileFrontendVersion: this.lockfileFrontendVersion,
       directDependencyVersions: this.frontendVersions,
       versionDoubt: this.versionDoubt,
       builtByGovernment: this.builtByGovernment,

helpers/repo-data.test.mjs

@@ -1,10 +1,5 @@
 import { describe, it, expect, vi } from 'vitest'
-import {
-  RepoData,
-  NoMetaDataError,
-  NoRepoTreeError,
-  NoCommitsError,
-} from './repo-data.mjs'
+import { RepoData, NoMetaDataError, NoRepoTreeError, NoCommitsError, UnsupportedLockFileError } from './repo-data.mjs'
 import {
   getFileContent,
   getLatestCommit,
@@ -93,16 +88,16 @@ describe('RepoData', () => {
   })
 
   describe('fetchAndValidateRepoTree', () => {
-    it('should throw a NoRepoTreeError if metadata is missing', async () => {
+    it('should throw a NoRepoTreeError if repo tree is missing', async () => {
       const repoData = new RepoData(repoOwner, repoName, serviceOwners)
-      getRepoMetaData.mockResolvedValue({
+      vi.spyOn(repoData, 'getLatestCommitSha').mockResolvedValue('test-sha')
+      getRepoTree.mockResolvedValue({
         data: {
-          pushed_at: null,
-          created_at: null,
+          tree: null,
         },
       })
 
-      await expect(repoData.fetchAndValidateMetaData()).rejects.toThrow(
+      await expect(repoData.fetchAndValidateRepoTree()).rejects.toThrow(
         NoRepoTreeError
       )
     })
@@ -177,6 +172,31 @@ describe('RepoData', () => {
     })
   })
 
+  describe('getLockfileType', () => {
+    it('should return package-lock.json if it exists', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      repoData.repoTree = { data: { tree: [{ path: 'package-lock.json' }] } }
+
+      const lockfileType = repoData.getLockfileType()
+      expect(lockfileType).toBe('package-lock.json')
+    })
+
+    it('should return yarn.lock if it exists', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      repoData.repoTree = { data: { tree: [{ path: 'yarn.lock' }] } }
+
+      const lockfileType = repoData.getLockfileType()
+      expect(lockfileType).toBe('yarn.lock')
+    })
+
+    it('should throw UnsupportedLockFileError if no supported lockfile exists', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      repoData.repoTree = { data: { tree: [{ path: 'other-file.lock' }] } }
+
+      expect(() => repoData.getLockfileType()).toThrow(UnsupportedLockFileError)
+    })
+  })
+
   describe('getAllFilesContent', () => {
     it('should get the content of all files with a given name', async () => {
       const repoData = new RepoData(repoOwner, repoName, serviceOwners)
@@ -200,6 +220,174 @@ describe('RepoData', () => {
       ])
     })
   })
+  describe('checkDirectDependency', () => {
+    it('should detect direct dependency on govuk-frontend', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const packageObjects = [
+        {
+          content: { dependencies: { 'govuk-frontend': '3.11.0' } },
+          path: 'package.json',
+        },
+      ]
+
+      const hasDirectDependency = repoData.checkDirectDependency(packageObjects)
+      expect(hasDirectDependency).toBe(true)
+      expect(repoData.frontendVersions).toEqual([
+        { packagePath: 'package.json', frontendVersion: '3.11.0' },
+      ])
+    })
+
+    it('should detect indirect dependency on govuk-frontend', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const packageObjects = [
+        {
+          content: { dependencies: { 'other-dependency': '1.0.0' } },
+          path: 'package.json',
+        },
+      ]
+
+      const hasDirectDependency = repoData.checkDirectDependency(packageObjects)
+      expect(hasDirectDependency).toBe(false)
+      expect(repoData.indirectDependency).toBe(true)
+    })
+  })
+  describe('getVersionFromLockfile', () => {
+    it('should get version from package-lock.json if ambiguous version in package.json', async () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      repoData.frontendVersions = [{ packagePath: 'package.json', frontendVersion: '3.11.0' }]
+      const lockfileContent = {
+        data: JSON.stringify({
+          packages: {
+            'node_modules/govuk-frontend': { version: '3.11.0' },
+          },
+        }),
+      }
+      getFileContent.mockResolvedValue(lockfileContent)
+
+      const version = await repoData.getVersionFromLockfile('package-lock.json')
+      expect(version).toBe('3.11.0')
+    })
+
+    it('should get version from package-lock.json if no versions in package.json', async () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const lockfileContent = {
+        data: JSON.stringify(
+          {
+            packages: {
+              'parent-dependency': {
+                dependencies: {
+                  'govuk-frontend': { version: '3.11.0' },
+                },
+              }
+            }
+          }),
+      }
+      getFileContent.mockResolvedValue(lockfileContent)
+
+      const version = await repoData.getVersionFromLockfile('package-lock.json')
+      expect(version).toBe('3.11.0')
+    })
+
+    it('should get version from yarn.lock', async () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const lockfileContent = {
+        data: `
+govuk-frontend@^3.11.0:
+  version "3.11.0"
+  resolved "https://registry.yarnpkg.com/govuk-frontend/-/govuk-frontend-3.11.0.tgz#hash"
+  integrity sha512-hash
+`
+      }
+      getFileContent.mockResolvedValue(lockfileContent)
+
+      const version = await repoData.getVersionFromLockfile('yarn.lock')
+      expect(version).toBe('3.11.0')
+    })
+  })
+  describe('handleError', () => {
+    it('should log NoMetaDataError', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const consoleSpy = vi.spyOn(console, 'log')
+      const error = new NoMetaDataError()
+
+      repoData.handleError(error)
+      expect(consoleSpy).toHaveBeenCalledWith(
+        "test-owner/test-repo: ERROR: couldn't fetch metadata"
+      )
+    })
+
+    it('should log NoCommitsError', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const consoleSpy = vi.spyOn(console, 'log')
+      const error = new NoCommitsError()
+
+      repoData.handleError(error)
+      expect(consoleSpy).toHaveBeenCalledWith(
+        "test-owner/test-repo: ERROR: couldn't fetch repo tree as repo has no commits"
+      )
+    })
+
+    it('should log NoRepoTreeError', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const consoleSpy = vi.spyOn(console, 'log')
+      const error = new NoRepoTreeError()
+
+      repoData.handleError(error)
+      expect(consoleSpy).toHaveBeenCalledWith(
+        "test-owner/test-repo: ERROR: couldn't fetch repo tree"
+      )
+    })
+
+    it('should log UnsupportedLockFileError', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const consoleSpy = vi.spyOn(console, 'log')
+      const error = new UnsupportedLockFileError()
+
+      repoData.handleError(error)
+      expect(consoleSpy).toHaveBeenCalledWith(
+        "test-owner/test-repo: ERROR: couldn't find a supported lockfile. Skipping version check."
+      )
+    })
+
+    it('should rethrow unknown errors', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      const error = new Error('Unknown error')
+
+      expect(() => repoData.handleError(error)).toThrow('Unknown error')
+    })
+  })
+  describe('getResult', () => {
+    it('should return the result of the analysis', () => {
+      const repoData = new RepoData(repoOwner, repoName, serviceOwners)
+      repoData.couldntAccess = true
+      repoData.lockfileFrontendVersion = '3.11.0'
+      repoData.versionDoubt = true
+      repoData.builtByGovernment = true
+      repoData.indirectDependency = true
+      repoData.isPrototype = true
+      repoData.lastUpdated = '2023-01-01T00:00:00Z'
+      repoData.repoCreated = '2022-01-01T00:00:00Z'
+      repoData.parentDependency = [{ parent: 'test-parent', version: '1.0.0' }]
+      repoData.errorThrown = 'Some error'
+
+      const result = repoData.getResult()
+      expect(result).toEqual({
+        repoOwner: 'test-owner',
+        repoName: 'test-repo',
+        couldntAccess: true,
+        lockfileFrontendVersion: '3.11.0',
+        directDependencyVersions: [],
+        versionDoubt: true,
+        builtByGovernment: true,
+        indirectDependency: true,
+        isPrototype: true,
+        lastUpdated: '2023-01-01T00:00:00Z',
+        repoCreated: '2022-01-01T00:00:00Z',
+        parentDependency: [{ parent: 'test-parent', version: '1.0.0' }],
+        errorThrown: 'Some error',
+      })
+    })
+  })
 
   it('should get the content of a file in the repo', async () => {
     const repoData = new RepoData(repoOwner, repoName, serviceOwners)

todo.md

@@ -36,3 +36,7 @@
   - Number of errored checks
   - Number of direct dependencies
   - Number of indirect dependencies
+
+# Notes
+- Investigated using dependency graph, but there's no easy way to get dependents and trees
+- Investigated using search API, but rate limit for code search is 10 per minute (and normal search is 30 per minute)