add more unit tests for catching invalid lockfiles early

nix-community · Oct 13, 2023 · 2760d68 · 2760d68
1 parent fe14519
commit 2760d68
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 62 deletions.
diff --git a/lib/internal/nodejsLockUtils.nix b/lib/internal/nodejsLockUtils.nix
@@ -9,14 +9,21 @@
 
   sanitizeLockfile = lock:
   # Every project MUST have a name
-  assert lock ? name;
-  # Every project MUST have a version
-  assert lock ? version;
-  # This lockfile module only supports lockfileVersion 2 and 3
-  assert !lock ? lockfileVersion || lock.lockfileVersion >= 2;
-  # The Lockfile must contain a 'packages' attribute.
-  assert lock ? packages;
-  lock;
+    if ! lock ? name
+    then throw "Invalid lockfile: Every project MUST have a name"
+    else
+      # Every project MUST have a version
+      if ! lock ? version
+      then throw "Invalid lockfile: Every project MUST have a version"
+      else
+        # This lockfile module only supports lockfileVersion 2 and 3
+        if ! lock ? lockfileVersion || lock.lockfileVersion <= 1
+        then throw "This lockfile module only supports lockfileVersion 2 and 3"
+        else
+          # The Lockfile must contain a 'packages' attribute.
+          if ! lock ? packages
+          then throw "Invalid lockfile: The Lockfile must contain 'packages' attribute."
+          else lock;
 
   findEntry =
     # = "attrs"
@@ -34,33 +41,6 @@
       else if currentPath == ""
       then throw "${search} not found in package-lock.json."
       else findEntry packageLock (stripPath currentPath) search;
-
-   # Returns the names of all "bundledDependencies".
-   # People depend on different types and different names. Unfortunatly those fields are not part of the offical npm documentation.
-   # Which may also be the reason for the mess.
-   # 
-   # TODO: define unit tests.
-   # Adopted from https://github.com/aakropotkin/floco/blob/708c4ffa0c05033c29fe6886a238cb20c3ba3fb4/modules/plock/implementation.nix#L139
-   # 
-   # getBundledDependencies :: Pent -> {}
-   getBundledDependencies = pent: let
-        # b :: bool | []
-        b = pent.bundledDependencies or pent.bundleDependencies or [];
-    in  
-        # The following asserts is the XOR logic.
-        # "bundle" and "bundled" dependencies are both valid but invalid if both or none keys exist
-        assert     ( pent ? bundledDependencies ) ->
-              ( ! ( pent ? bundleDependencies  ) );
-        assert     ( pent ? bundleDependencies  ) ->
-              ( ! ( pent ? bundledDependencies ) );
-        if b == [] then {} else
-        if builtins.isList b then { bundledDependencies = b; } else
-        if ! b then {} else {
-        # b :: true
-        bundledDependencies = builtins.attrNames (
-          ( pent.dependencies or {} ) // ( pent.requires or {} )
-        );
-    };
 in {
-  inherit findEntry stripPath getBundledDependencies sanitizeLockfile;
+  inherit findEntry stripPath sanitizeLockfile;
 }
diff --git a/modules/dream2nix/nodejs-package-lock-v3/default.nix b/modules/dream2nix/nodejs-package-lock-v3/default.nix
@@ -9,7 +9,7 @@
 
   inherit (config.deps) fetchurl;
 
-  nodejsLockUtils = import ../../../lib/internal/nodejsLockUtils.nix { inherit lib; };
+  nodejsLockUtils = import ../../../lib/internal/nodejsLockUtils.nix {inherit lib;};
 
   # Collection of sanitized functions that always return the same type
   isLink = pent: pent.link or false;
@@ -21,10 +21,11 @@
   # getBin = pent: pent.bin or {};
 
   /*
-    Pent :: {
-      See: https://docs.npmjs.com/cli/v9/configuring-npm/package-lock-json#packages
-    }
-    pent is one entry of 'packages'
+  Pent :: {
+    See: https://docs.npmjs.com/cli/v9/configuring-npm/package-lock-json#packages
+  }
+  > We should mention that docs are imcomplete on npmjs.com
+  pent is one entry of 'packages'
   */
   parseSource = pent:
     if isLink pent
@@ -37,20 +38,16 @@
         hash = pent.integrity;
       };
 
-
   getDependencies = lock: path: pent:
-    l.mapAttrs (depName: _semverConstraint: 
-    let
+    l.mapAttrs (depName: _semverConstraint: let
       packageIdent = nodejsLockUtils.findEntry lock path depName;
       depPent = lock.packages.${packageIdent};
-    in
-    {
+    in {
       dev = pent.dev or false;
       version = depPent.version;
     })
     (pent.dependencies or {} // pent.devDependencies or {} // pent.optionalDependencies or {});
 
-
   # Takes one entry of "package" from package-lock.json
   parseEntry = lock: path: entry:
     if path == ""
@@ -84,7 +81,8 @@
         };
       };
 
-  mergePdefs = builtins.foldl'
+  mergePdefs =
+    builtins.foldl'
     (acc: entry:
       acc
       // {

diff --git a/modules/dream2nix/nodejs-package-lock-v3/interface.nix b/modules/dream2nix/nodejs-package-lock-v3/interface.nix
@@ -71,7 +71,7 @@ in {
     /*
 
     type: pdefs.${name}.${version} :: {
-      
+
       // Pointing to the source of the package.
       // in most cases this is a tarball (tar.gz) which needs to be unpacked by e.g. unpackPhase
       source :: Derivation | Path
@@ -86,7 +86,7 @@ in {
         }
       }
     }
-   */ 
+    */
     pdefs = {
       type = t.attrsOf (t.attrsOf (t.submodule {
         options.dependencies = l.mkOption {

diff --git a/tests/nix-unit/test_nodejs_lock_v3/default.nix b/tests/nix-unit/test_nodejs_lock_v3/default.nix
@@ -297,18 +297,16 @@ in {
       imports = [
         dream2nix.modules.dream2nix.nodejs-package-lock-v3
       ];
-      nodejs-package-lock-v3.packageLock = lib.mkForce {
-        # Example content of lockfile
-        # "lockfileVersion" = 1;
-      };
+      nodejs-package-lock-v3.packageLock =
+        lib.mkForce {
+        };
     };
     config = evaled.config;
   in {
-    expr = builtins.tryEval (config.nodejs-package-lock-v3.pdefs);
-    expected =  {
-      success = false;
-      value = false;
+    expr = config.nodejs-package-lock-v3.pdefs;
+    expectedError = {
+      type = "ThrownError";
+      msg = "Invalid lockfile";
     };
   };
-
 }
diff --git a/tests/nix-unit/test_nodejs_lockutils/default.nix b/tests/nix-unit/test_nodejs_lockutils/default.nix
@@ -74,8 +74,8 @@
     expr = path;
     expected = "node_modules/underscore";
   };
-  
-  # test the lock 
+
+  # test the lock
   test_nodejsLockUtils_lockfile_v3 = let
     plock = {
       name = "foo";
@@ -110,7 +110,69 @@
     };
   in {
     expr = nodejsLockUtils.sanitizeLockfile plock;
-    expectedError = plock;
+    expectedError = {
+      type = "ThrownError";
+      msg = "This lockfile module only supports lockfileVersion 2 and 3";
+    };
   };
 
+  test_nodejsLockUtils_lockfile_missing_name = let
+    plock = {
+      # name = "foo";
+      version = "1.0.0";
+      lockfileVersion = 3;
+      packages = {};
+    };
+  in {
+    expr = nodejsLockUtils.sanitizeLockfile plock;
+    expectedError = {
+      type = "ThrownError";
+      msg = "MUST have a name";
+    };
+  };
+
+  test_nodejsLockUtils_lockfile_missing_version = let
+    plock = {
+      name = "foo";
+      # version = "1.0.0";
+      lockfileVersion = 3;
+      packages = {};
+    };
+  in {
+    expr = nodejsLockUtils.sanitizeLockfile plock;
+    expectedError = {
+      type = "ThrownError";
+      msg = "MUST have a version";
+    };
+  };
+
+  test_nodejsLockUtils_lockfile_missing_lockfileVersion = let
+    plock = {
+      name = "foo";
+      version = "1.0.0";
+      # lockfileVersion = 3;
+      packages = {};
+    };
+  in {
+    expr = nodejsLockUtils.sanitizeLockfile plock;
+    expectedError = {
+      type = "ThrownError";
+      msg = "lockfileVersion";
+    };
+  };
+
+  test_nodejsLockUtils_lockfile_missing_packages = let
+    plock = {
+      name = "foo";
+      version = "1.0.0";
+      lockfileVersion = 3;
+      # packages = {};
+    };
+  in {
+    expr = nodejsLockUtils.sanitizeLockfile plock;
+    expectedError = {
+      type = "ThrownError";
+      msg = "must contain 'packages'";
+    };
+  };
 }