Merge pull request #957 from Sh4rK/pin-bundled

Fix pinning of npm@bundled

Author: charlespierce

crates/volta-core/src/platform/mod.rs

@@ -244,34 +244,17 @@ impl Platform {
     ///   pulling Yarn from the user default platform, if available
     /// - If there is no Project platform, then we use the user Default Platform
     pub fn current(session: &mut Session) -> Fallible<Option<Self>> {
-        match session.project_platform()? {
-            Some(platform) => {
-                if platform.yarn.is_none() || platform.npm.is_none() {
-                    if let Some(default) = session.default_platform()? {
-                        let npm = platform
-                            .npm
-                            .clone()
-                            .map(Sourced::with_project)
-                            .or_else(|| default.npm.clone().map(Sourced::with_default));
-                        let yarn = platform
-                            .yarn
-                            .clone()
-                            .map(Sourced::with_project)
-                            .or_else(|| default.yarn.clone().map(Sourced::with_default));
-
-                        return Ok(Some(Platform {
-                            node: Sourced::with_project(platform.node.clone()),
-                            npm,
-                            yarn,
-                        }));
-                    }
-                }
-                Ok(Some(platform.as_project()))
+        if let Some(mut platform) = session.project_platform()?.map(PlatformSpec::as_project) {
+            if platform.yarn.is_none() {
+                platform.yarn = session
+                    .default_platform()?
+                    .and_then(|default_platform| default_platform.yarn.clone())
+                    .map(Sourced::with_default);
             }
-            None => match session.default_platform()? {
-                Some(platform) => Ok(Some(platform.as_default())),
-                None => Ok(None),
-            },
+
+            Ok(Some(platform))
+        } else {
+            Ok(session.default_platform()?.map(PlatformSpec::as_default))
         }
     }
 

tests/acceptance/merged_platform.rs

@@ -1,10 +1,25 @@
-use crate::support::sandbox::{sandbox, DistroMetadata, NodeFixture, YarnFixture};
+use crate::support::sandbox::{sandbox, DistroMetadata, NodeFixture, NpmFixture, YarnFixture};
 use hamcrest2::assert_that;
 use hamcrest2::prelude::*;
 use test_support::matchers::execs;
 
 use volta_core::error::ExitCode;
 
+const PACKAGE_JSON_NODE_ONLY: &str = r#"{
+    "name": "node-only",
+    "volta": {
+        "node": "10.99.1040"
+    }
+}"#;
+
+const PACKAGE_JSON_WITH_NPM: &str = r#"{
+    "name": "with-npm",
+    "volta": {
+        "node": "10.99.1040",
+        "npm": "4.5.6"
+    }
+}"#;
+
 const PACKAGE_JSON_WITH_YARN: &str = r#"{
     "name": "with-yarn",
     "volta": {
@@ -13,26 +28,26 @@ const PACKAGE_JSON_WITH_YARN: &str = r#"{
     }
 }"#;
 
-const PACKAGE_JSON_NO_YARN: &str = r#"{
-    "name": "without-yarn",
-    "volta": {
-        "node": "10.99.1040"
+const PLATFORM_NODE_ONLY: &str = r#"{
+    "node":{
+        "runtime":"9.27.6",
+        "npm":null
     }
 }"#;
 
-const PLATFORM_WITH_YARN: &str = r#"{
+const PLATFORM_WITH_NPM: &str = r#"{
     "node":{
         "runtime":"9.27.6",
-        "npm":null
-    },
-    "yarn": "1.7.71"
+        "npm":"1.2.3"
+    }
 }"#;
 
-const PLATFORM_NO_YARN: &str = r#"{
+const PLATFORM_WITH_YARN: &str = r#"{
     "node":{
         "runtime":"9.27.6",
         "npm":null
-    }
+    },
+    "yarn": "1.7.71"
 }"#;
 
 cfg_if::cfg_if! {
@@ -80,6 +95,19 @@ cfg_if::cfg_if! {
     }
 }
 
+const NPM_VERSION_FIXTURES: [DistroMetadata; 2] = [
+    DistroMetadata {
+        version: "1.2.3",
+        compressed_size: 239,
+        uncompressed_size: Some(0x0028_0000),
+    },
+    DistroMetadata {
+        version: "4.5.6",
+        compressed_size: 239,
+        uncompressed_size: Some(0x0028_0000),
+    },
+];
+
 const YARN_VERSION_FIXTURES: [DistroMetadata; 2] = [
     DistroMetadata {
         version: "1.12.99",
@@ -93,6 +121,80 @@ const YARN_VERSION_FIXTURES: [DistroMetadata; 2] = [
     },
 ];
 
+#[test]
+fn uses_project_npm_if_available() {
+    let s = sandbox()
+        .platform(PLATFORM_WITH_NPM)
+        .package_json(PACKAGE_JSON_WITH_NPM)
+        .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
+        .distro_mocks::<NpmFixture>(&NPM_VERSION_FIXTURES)
+        .env("VOLTA_LOGLEVEL", "debug")
+        .build();
+
+    assert_that!(
+        s.npm("--version"),
+        execs()
+            .with_status(ExitCode::Success as i32)
+            .with_stderr_contains("[..]Node: 10.99.1040 from project configuration")
+            .with_stderr_contains("[..]npm: 4.5.6 from project configuration")
+    );
+}
+
+#[test]
+fn uses_bundled_npm_in_project_without_npm() {
+    let s = sandbox()
+        .platform(PLATFORM_WITH_NPM)
+        .package_json(PACKAGE_JSON_NODE_ONLY)
+        .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
+        .distro_mocks::<NpmFixture>(&NPM_VERSION_FIXTURES)
+        .env("VOLTA_LOGLEVEL", "debug")
+        .build();
+
+    assert_that!(
+        s.npm("--version"),
+        execs()
+            .with_status(ExitCode::Success as i32)
+            .with_stderr_contains("[..]Node: 10.99.1040 from project configuration")
+            .with_stderr_contains("[..]npm: 6.2.26 from project configuration")
+    );
+}
+
+#[test]
+fn uses_default_npm_outside_project() {
+    let s = sandbox()
+        .platform(PLATFORM_WITH_NPM)
+        .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
+        .distro_mocks::<NpmFixture>(&NPM_VERSION_FIXTURES)
+        .env("VOLTA_LOGLEVEL", "debug")
+        .build();
+
+    assert_that!(
+        s.npm("--version"),
+        execs()
+            .with_status(ExitCode::Success as i32)
+            .with_stderr_contains("[..]Node: 9.27.6 from default configuration")
+            .with_stderr_contains("[..]npm: 1.2.3 from default configuration")
+    );
+}
+
+#[test]
+fn uses_bundled_npm_outside_project() {
+    let s = sandbox()
+        .platform(PLATFORM_NODE_ONLY)
+        .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
+        .distro_mocks::<NpmFixture>(&NPM_VERSION_FIXTURES)
+        .env("VOLTA_LOGLEVEL", "debug")
+        .build();
+
+    assert_that!(
+        s.npm("--version"),
+        execs()
+            .with_status(ExitCode::Success as i32)
+            .with_stderr_contains("[..]Node: 9.27.6 from default configuration")
+            .with_stderr_contains("[..]npm: 5.6.17 from default configuration")
+    );
+}
+
 #[test]
 fn uses_project_yarn_if_available() {
     let s = sandbox()
@@ -117,7 +219,7 @@ fn uses_project_yarn_if_available() {
 fn uses_default_yarn_in_project_without_yarn() {
     let s = sandbox()
         .platform(PLATFORM_WITH_YARN)
-        .package_json(PACKAGE_JSON_NO_YARN)
+        .package_json(PACKAGE_JSON_NODE_ONLY)
         .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
         .distro_mocks::<YarnFixture>(&YARN_VERSION_FIXTURES)
         .env("VOLTA_LOGLEVEL", "debug")
@@ -155,8 +257,8 @@ fn uses_default_yarn_outside_project() {
 #[test]
 fn throws_project_error_in_project() {
     let s = sandbox()
-        .platform(PLATFORM_NO_YARN)
-        .package_json(PACKAGE_JSON_NO_YARN)
+        .platform(PLATFORM_NODE_ONLY)
+        .package_json(PACKAGE_JSON_NODE_ONLY)
         .build();
 
     assert_that!(
@@ -169,7 +271,7 @@ fn throws_project_error_in_project() {
 
 #[test]
 fn throws_default_error_outside_project() {
-    let s = sandbox().platform(PLATFORM_NO_YARN).build();
+    let s = sandbox().platform(PLATFORM_NODE_ONLY).build();
 
     assert_that!(
         s.yarn("--version"),