From a3a81c2322470a991e06a54d1d89fb566e4a636b Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Wed, 27 Dec 2023 13:17:22 +0100 Subject: [PATCH 1/8] Trivial: Shorten a few parameter names The parameter names were repeating the type name, which in a language like D does not make sense. --- source/dub/dub.d | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/source/dub/dub.d b/source/dub/dub.d index e7c54043a..ddeaa07cf 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -135,22 +135,23 @@ class Dub { Params: root_path = Path to the root package - additional_package_suppliers = A list of package suppliers to try - before the suppliers found in the configurations files and the - `defaultPackageSuppliers`. - skip_registry = Can be used to skip using the configured package - suppliers, as well as the default suppliers. + base = A list of package suppliers that are always present + (regardless of `skip`) and take precedence over the default + and configured `PackageSupplier`. This setting is currently + not used by the dub application but useful for libraries. + skip = Can be used to skip using the configured package suppliers, + as well as the default suppliers. */ - this(string root_path = ".", PackageSupplier[] additional_package_suppliers = null, - SkipPackageSuppliers skip_registry = SkipPackageSuppliers.none) + this(string root_path = ".", PackageSupplier[] base = null, + SkipPackageSuppliers skip = SkipPackageSuppliers.none) { m_rootPath = NativePath(root_path); if (!m_rootPath.absolute) m_rootPath = getWorkingDirectory() ~ m_rootPath; init(); - m_packageSuppliers = this.computePkgSuppliers(additional_package_suppliers, - skip_registry, environment.get("DUB_REGISTRY", null)); + const registry_var = environment.get("DUB_REGISTRY", null); + m_packageSuppliers = this.computePkgSuppliers(base, skip, registry_var); m_packageManager = new PackageManager(m_rootPath, m_dirs.userPackages, m_dirs.systemSettings, false); auto ccps = m_config.customCachePaths; @@ -298,40 +299,39 @@ class Dub { additional_package_suppliers = A list of package suppliers to try before the suppliers found in the configurations files and the `defaultPackageSuppliers`. - skip_registry = Can be used to skip using the configured package - suppliers, as well as the default suppliers. + skip = Can be used to skip using the configured package suppliers, + as well as the default suppliers. */ deprecated("This is an implementation detail. " ~ "Use `packageSuppliers` to get the computed list of package " ~ "suppliers once a `Dub` instance has been constructed.") - public PackageSupplier[] getPackageSuppliers(PackageSupplier[] additional_package_suppliers, SkipPackageSuppliers skip_registry) + public PackageSupplier[] getPackageSuppliers(PackageSupplier[] base, SkipPackageSuppliers skip) { - return this.computePkgSuppliers(additional_package_suppliers, skip_registry, environment.get("DUB_REGISTRY", null)); + return this.computePkgSuppliers(base, skip, environment.get("DUB_REGISTRY", null)); } /// Ditto - private PackageSupplier[] computePkgSuppliers( - PackageSupplier[] additional_package_suppliers, SkipPackageSuppliers skip_registry, - string dub_registry_var) + private PackageSupplier[] computePkgSuppliers(PackageSupplier[] base, + SkipPackageSuppliers skip, string registry_var) { - PackageSupplier[] ps = additional_package_suppliers; + PackageSupplier[] ps = base; - if (skip_registry < SkipPackageSuppliers.all) + if (skip < SkipPackageSuppliers.all) { - ps ~= dub_registry_var + ps ~= registry_var .splitter(";") .map!(url => getRegistryPackageSupplier(url)) .array; } - if (skip_registry < SkipPackageSuppliers.configured) + if (skip < SkipPackageSuppliers.configured) { ps ~= m_config.registryUrls .map!(url => getRegistryPackageSupplier(url)) .array; } - if (skip_registry < SkipPackageSuppliers.standard) + if (skip < SkipPackageSuppliers.standard) ps ~= defaultPackageSuppliers(); return ps; From 6a1a0eace76eb0a8555e0ccf7ac1b3ead67207cd Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Wed, 27 Dec 2023 13:19:29 +0100 Subject: [PATCH 2/8] Trivial: Rename computePkgSuppliers to makePackageSuppliers This is consistent with the naming of other factory functions. --- source/dub/dub.d | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/source/dub/dub.d b/source/dub/dub.d index ddeaa07cf..b078a4e1d 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -151,7 +151,7 @@ class Dub { init(); const registry_var = environment.get("DUB_REGISTRY", null); - m_packageSuppliers = this.computePkgSuppliers(base, skip, registry_var); + m_packageSuppliers = this.makePackageSuppliers(base, skip, registry_var); m_packageManager = new PackageManager(m_rootPath, m_dirs.userPackages, m_dirs.systemSettings, false); auto ccps = m_config.customCachePaths; @@ -307,11 +307,11 @@ class Dub { "suppliers once a `Dub` instance has been constructed.") public PackageSupplier[] getPackageSuppliers(PackageSupplier[] base, SkipPackageSuppliers skip) { - return this.computePkgSuppliers(base, skip, environment.get("DUB_REGISTRY", null)); + return this.makePackageSuppliers(base, skip, environment.get("DUB_REGISTRY", null)); } /// Ditto - private PackageSupplier[] computePkgSuppliers(PackageSupplier[] base, + private PackageSupplier[] makePackageSuppliers(PackageSupplier[] base, SkipPackageSuppliers skip, string registry_var) { PackageSupplier[] ps = base; @@ -352,10 +352,10 @@ class Dub { assert(dub.m_packageSuppliers.length == 3); dub = new TestDub(); - assert(dub.computePkgSuppliers(null, SkipPackageSuppliers.none, null).length == 1); - assert(dub.computePkgSuppliers(null, SkipPackageSuppliers.configured, null).length == 0); - assert(dub.computePkgSuppliers(null, SkipPackageSuppliers.standard, null).length == 0); - assert(dub.computePkgSuppliers(null, SkipPackageSuppliers.standard, "http://example.com/") + assert(dub.makePackageSuppliers(null, SkipPackageSuppliers.none, null).length == 1); + assert(dub.makePackageSuppliers(null, SkipPackageSuppliers.configured, null).length == 0); + assert(dub.makePackageSuppliers(null, SkipPackageSuppliers.standard, null).length == 0); + assert(dub.makePackageSuppliers(null, SkipPackageSuppliers.standard, "http://example.com/") .length == 1); } From 0fdbca3b558578dcc0bb172b45a643497b99c93e Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Wed, 27 Dec 2023 13:21:09 +0100 Subject: [PATCH 3/8] Trivial: Use public interface, not field names, in unittest This would make moving this unittest to another module easier. --- source/dub/dub.d | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/dub/dub.d b/source/dub/dub.d index b078a4e1d..d63693e52 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -341,15 +341,15 @@ class Dub { { scope (exit) environment.remove("DUB_REGISTRY"); auto dub = new TestDub(".", null, SkipPackageSuppliers.configured); - assert(dub.m_packageSuppliers.length == 0); + assert(dub.packageSuppliers.length == 0); environment["DUB_REGISTRY"] = "http://example.com/"; dub = new TestDub(".", null, SkipPackageSuppliers.configured); - assert(dub.m_packageSuppliers.length == 1); + assert(dub.packageSuppliers.length == 1); environment["DUB_REGISTRY"] = "http://example.com/;http://foo.com/"; dub = new TestDub(".", null, SkipPackageSuppliers.configured); - assert(dub.m_packageSuppliers.length == 2); + assert(dub.packageSuppliers.length == 2); dub = new TestDub(".", [new RegistryPackageSupplier(URL("http://bar.com/"))], SkipPackageSuppliers.configured); - assert(dub.m_packageSuppliers.length == 3); + assert(dub.packageSuppliers.length == 3); dub = new TestDub(); assert(dub.makePackageSuppliers(null, SkipPackageSuppliers.none, null).length == 1); From b0ae75865d6d5b1d63dc35f04c3ad0cc8797c373 Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Wed, 27 Dec 2023 13:23:42 +0100 Subject: [PATCH 4/8] Make inheriting from Dub easier This would make extending TestDub much easier by exposing the class fields, the common initialization routine, and a configuration point. It also allows us to move TestDub to another module. --- source/dub/dub.d | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/dub/dub.d b/source/dub/dub.d index d63693e52..bcb21bb78 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -107,7 +107,7 @@ unittest the command line interface. */ class Dub { - private { + protected { bool m_dryRun = false; PackageManager m_packageManager; PackageSupplier[] m_packageSuppliers; @@ -195,7 +195,7 @@ class Dub { this(pkg_root, pkg_root); } - private void init() + protected void init() { this.m_dirs = SpecialDirs.make(); this.m_config = this.loadConfig(this.m_dirs); @@ -311,7 +311,7 @@ class Dub { } /// Ditto - private PackageSupplier[] makePackageSuppliers(PackageSupplier[] base, + protected PackageSupplier[] makePackageSuppliers(PackageSupplier[] base, SkipPackageSuppliers skip, string registry_var) { PackageSupplier[] ps = base; @@ -1856,7 +1856,7 @@ version(unittest) package class TestDub : Dub } } -private struct SpecialDirs { +package struct SpecialDirs { /// The path where to store temporary files and directory NativePath temp; /// The system-wide dub-specific folder From 13f81324d526d191fa6f634a40819e6fccae22cc Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Wed, 27 Dec 2023 13:32:11 +0100 Subject: [PATCH 5/8] Move TestDub to its own module As we will begin to inject more dependencies, having a base module makes sense. --- source/dub/dub.d | 32 +++++++------------------ source/dub/test/base.d | 53 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 source/dub/test/base.d diff --git a/source/dub/dub.d b/source/dub/dub.d index bcb21bb78..fe5efe4c1 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -337,8 +337,12 @@ class Dub { return ps; } + // Note: This test rely on the environment, which is not how unittests should work. + // This should be removed / refactored to keep coverage without affecting the env. unittest { + import dub.test.base : TestDub; + scope (exit) environment.remove("DUB_REGISTRY"); auto dub = new TestDub(".", null, SkipPackageSuppliers.configured); assert(dub.packageSuppliers.length == 0); @@ -1458,9 +1462,13 @@ class Dub { return compilers[0]; } + // This test also relies on the environment and the filesystem, + // as the `makePackageSuppliers` does, and should be refactored. unittest { + import dub.test.base : TestDub; import std.path: buildPath, absolutePath; + auto dub = new TestDub(".", null, SkipPackageSuppliers.configured); immutable olddc = environment.get("DC", null); immutable oldpath = environment.get("PATH", null); @@ -1832,30 +1840,6 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend } } -/** - * An instance of Dub that does not rely on the environment - * - * This instance of dub should not read any environment variables, - * nor should it do any file IO, to make it usable and reliable in unittests. - * Currently it reads environment variables but does not read the configuration. - */ -version(unittest) package class TestDub : Dub -{ - /// Forward to base constructor - public this (string root = ".", PackageSupplier[] extras = null, - SkipPackageSuppliers skip = SkipPackageSuppliers.none) - { - super(root, extras, skip); - } - - /// Avoid loading user configuration - protected override Settings loadConfig(ref SpecialDirs dirs) const - { - // No-op - return Settings.init; - } -} - package struct SpecialDirs { /// The path where to store temporary files and directory NativePath temp; diff --git a/source/dub/test/base.d b/source/dub/test/base.d new file mode 100644 index 000000000..6f7c7b3b9 --- /dev/null +++ b/source/dub/test/base.d @@ -0,0 +1,53 @@ +/******************************************************************************* + + Base utilities (types, functions) used in tests + +*******************************************************************************/ + +module dub.test.base; + +version (unittest): + +import dub.data.settings; +public import dub.dependency; +import dub.dub; +import dub.package_; +import dub.packagemanager; +import dub.packagesuppliers.packagesupplier; + +// TODO: Remove and handle logging the same way we handle other IO +import dub.internal.logging; + +public void enableLogging() +{ + setLogLevel(LogLevel.debug_); +} + +public void disableLogging() +{ + setLogLevel(LogLevel.none); +} + +/** + * An instance of Dub that does not rely on the environment + * + * This instance of dub should not read any environment variables, + * nor should it do any file IO, to make it usable and reliable in unittests. + * Currently it reads environment variables but does not read the configuration. + */ +public class TestDub : Dub +{ + /// Forward to base constructor + public this (string root = ".", PackageSupplier[] extras = null, + SkipPackageSuppliers skip = SkipPackageSuppliers.none) + { + super(root, extras, skip); + } + + /// Avoid loading user configuration + protected override Settings loadConfig(ref SpecialDirs dirs) const + { + // No-op + return Settings.init; + } +} From 1f7a6dd2fef1cafffabb356b90791c030b2e2291 Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Tue, 26 Dec 2023 21:39:55 +0100 Subject: [PATCH 6/8] Allow dependency injection on the PackageSupplier In order to make the `Dub` class properly unittest-able, we need to be able to use a non-default `PackageSupplier`, in particular one that would not hit the registry or the filesystem. This can be achieved by moving a free function to a method and having derived class override it, as shown in this commit. --- source/dub/dub.d | 40 +++++++++++++++++++++--- source/dub/test/base.d | 71 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/source/dub/dub.d b/source/dub/dub.d index fe5efe4c1..128b1672c 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -60,6 +60,7 @@ static immutable string[] defaultRegistryURLs = [ See_Also: `defaultRegistryURLs` */ +deprecated("This function wasn't intended for public use - open an issue with Dub if you need it") PackageSupplier[] defaultPackageSuppliers() { logDiagnostic("Using dub registry url '%s'", defaultRegistryURLs[0]); @@ -70,6 +71,7 @@ PackageSupplier[] defaultPackageSuppliers() Allowed protocols are dub+http(s):// and maven+http(s)://. */ +deprecated("This function wasn't intended for public use - open an issue with Dub if you need it") PackageSupplier getRegistryPackageSupplier(string url) { switch (url.startsWith("dub+", "mvn+", "file://")) @@ -85,7 +87,7 @@ PackageSupplier getRegistryPackageSupplier(string url) } } -unittest +deprecated unittest { auto dubRegistryPackageSupplier = getRegistryPackageSupplier("dub+https://code.dlang.org"); assert(dubRegistryPackageSupplier.description.canFind(" https://code.dlang.org")); @@ -320,19 +322,21 @@ class Dub { { ps ~= registry_var .splitter(";") - .map!(url => getRegistryPackageSupplier(url)) + .map!(url => this.makePackageSupplier(url)) .array; } if (skip < SkipPackageSuppliers.configured) { ps ~= m_config.registryUrls - .map!(url => getRegistryPackageSupplier(url)) + .map!(url => this.makePackageSupplier(url)) .array; } if (skip < SkipPackageSuppliers.standard) - ps ~= defaultPackageSuppliers(); + ps ~= new FallbackPackageSupplier( + defaultRegistryURLs.map!(url => this.makePackageSupplier(url)) + .array); return ps; } @@ -363,6 +367,34 @@ class Dub { .length == 1); } + /** + * Instantiate a `PackageSupplier` according to a given URL + * + * This is a factory function for `PackageSupplier`. Child classes may + * wish to override this to implement their own `PackageSupplier` logic, + * be it by extending this method's ability or replacing it. + * + * Params: + * url = The URL of the `PackageSupplier`. + * + * Returns: + * A new instance of a `PackageSupplier`. + */ + protected PackageSupplier makePackageSupplier(string url) const + { + switch (url.startsWith("dub+", "mvn+", "file://")) + { + case 1: + return new RegistryPackageSupplier(URL(url[4..$])); + case 2: + return new MavenRegistryPackageSupplier(URL(url[4..$])); + case 3: + return new FileSystemPackageSupplier(NativePath(url[7..$])); + default: + return new RegistryPackageSupplier(URL(url)); + } + } + /// ditto deprecated("This is an implementation detail. " ~ "Use `packageSuppliers` to get the computed list of package " ~ diff --git a/source/dub/test/base.d b/source/dub/test/base.d index 6f7c7b3b9..4e498762a 100644 --- a/source/dub/test/base.d +++ b/source/dub/test/base.d @@ -8,6 +8,9 @@ module dub.test.base; version (unittest): +import std.array; +public import std.algorithm; + import dub.data.settings; public import dub.dependency; import dub.dub; @@ -50,4 +53,72 @@ public class TestDub : Dub // No-op return Settings.init; } + + /// See `MockPackageSupplier` documentation for this class' implementation + protected override PackageSupplier makePackageSupplier(string url) const + { + return new MockPackageSupplier(url); + } +} + +/** + * Implements a `PackageSupplier` that doesn't do any IO + * + * This `PackageSupplier` needs to be pre-loaded with `Package` it can + * find during the setup phase of the unittest. + */ +public class MockPackageSupplier : PackageSupplier +{ + /// Mapping of package name to packages, ordered by `Version` + protected Package[][string] pkgs; + + /// URL this was instantiated with + protected string url; + + /// + public this(string url) + { + this.url = url; + } + + /// + public override @property string description() + { + return "unittest PackageSupplier for: " ~ this.url; + } + + /// + public override Version[] getVersions(string package_id) + { + if (auto ppkgs = package_id in this.pkgs) + return (*ppkgs).map!(pkg => pkg.version_).array; + return null; + } + + /// + public override void fetchPackage( + NativePath path, string package_id, in VersionRange dep, bool pre_release) + { + assert(0, this.url ~ " - fetchPackage not implemented for: " ~ package_id); + } + + /// + public override Json fetchPackageRecipe( + string package_id, in VersionRange dep, bool pre_release) + { + import dub.recipe.json; + + if (auto ppkgs = package_id in this.pkgs) + foreach_reverse (pkg; *ppkgs) + if ((!pkg.version_.isPreRelease || pre_release) && + dep.matches(pkg.version_)) + return toJson(pkg.recipe); + return Json.init; + } + + /// + public override SearchResult[] searchPackages(string query) + { + assert(0, this.url ~ " - searchPackages not implemented for: " ~ query); + } } From fa3c593e7cc1faadf7d2b60a04c9f2a223bfb32c Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Fri, 22 Dec 2023 17:14:43 +0100 Subject: [PATCH 7/8] Allow dependency injection on the PackageManager We want to allow one to provide their own PackageManager instance. There are 3 standard ways of doing this: 1) Provide an instance in the constructor; 2) Provide a template parameter; 3) Use a hook that is called by the constructor; We are going with option 3 as the other two methods have clear downsides: Option 1 exposes too much of the implementation detail (even if it can be wrapped by another constructor) and makes the dependency to internal state hard / impossible (we need to instantiate SpecialDirs then load the config that may modify SpecialDirs). Option 2 is even worse as it exposes implementation details to the type and makes OOP impossible (e.g. method accepting a `PackageManager` need to be changed). Option 3 is the best compromise, as its main downside is that it requires to create a new `Dub` type, however that is already required due to the amount of configuration points that do IO (loadConfig, determineDefaultCompiler, computePkgSuppliers). --- source/dub/dub.d | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/source/dub/dub.d b/source/dub/dub.d index 128b1672c..51d75c954 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -154,7 +154,7 @@ class Dub { const registry_var = environment.get("DUB_REGISTRY", null); m_packageSuppliers = this.makePackageSuppliers(base, skip, registry_var); - m_packageManager = new PackageManager(m_rootPath, m_dirs.userPackages, m_dirs.systemSettings, false); + m_packageManager = this.makePackageManager(); auto ccps = m_config.customCachePaths; if (ccps.length) @@ -197,6 +197,19 @@ class Dub { this(pkg_root, pkg_root); } + /** + * Get the `PackageManager` instance to use for this `Dub` instance + * + * The `PackageManager` is a central component of `Dub` as it allows to + * store and retrieve packages from the file system. In unittests, or more + * generally in a library setup, one may wish to provide a custom + * implementation, which can be done by overriding this method. + */ + protected PackageManager makePackageManager() const + { + return new PackageManager(m_rootPath, m_dirs.userPackages, m_dirs.systemSettings, false); + } + protected void init() { this.m_dirs = SpecialDirs.make(); From 7089cf8073d811d7f3de38846a47f541e8032aa7 Mon Sep 17 00:00:00 2001 From: Mathias Lang Date: Tue, 19 Dec 2023 22:25:15 +0100 Subject: [PATCH 8/8] Add unittest framework and some dependency tests Dub has always been notoriously hard to test due to the amount of IO it does. Over the past few years, I have progressively refactored it to allow dependency injection to take place, which this finally put into action. By overriding the `PackageManager`, `PackageSupplier`, and a few strategic functions, we can start to unittest Dub's behavior solely in unittests. This should hopefully tremendously helps with adding regression tests for the package manager side of Dub (the build side still need work to have similar capabilities). --- source/dub/packagemanager.d | 25 ++++-- source/dub/project.d | 4 +- source/dub/test/base.d | 157 ++++++++++++++++++++++++++++++++- source/dub/test/dependencies.d | 133 ++++++++++++++++++++++++++++ 4 files changed, 310 insertions(+), 9 deletions(-) create mode 100644 source/dub/test/dependencies.d diff --git a/source/dub/packagemanager.d b/source/dub/packagemanager.d index 8b10cf998..5e05208d6 100644 --- a/source/dub/packagemanager.d +++ b/source/dub/packagemanager.d @@ -188,7 +188,7 @@ class PackageManager { * Returns: * A `Package` if one was found, `null` if none exists. */ - private Package lookup (string name, Version vers) { + protected Package lookup (string name, Version vers) { if (!this.m_initialized) this.refresh(); @@ -243,6 +243,21 @@ class PackageManager { return this.lookup(name, ver); } + /** + * Adds a `Package` to this `PackageManager` + * + * This is currently only available in unittests as it is a convenience + * function used by `TestDub`, but could be generalized once IO has been + * abstracted away from this class. + */ + version (unittest) Package add(Package pkg) + { + // See `PackageManager.addPackages` for inspiration. + assert(!pkg.subPackages.length, "Subpackages are not yet supported"); + this.m_internal.fromPath ~= pkg; + return pkg; + } + /// ditto deprecated("Use the overload that accepts a `Version` as second argument") Package getPackage(string name, string ver, bool enable_overrides = true) @@ -1121,7 +1136,7 @@ private enum LocalOverridesFilename = "local-overrides.json"; * Additionally, each location host a config file, * which is not managed by this module, but by dub itself. */ -private struct Location { +package struct Location { /// The absolute path to the root of the location NativePath packagePath; @@ -1364,7 +1379,7 @@ private struct Location { * Returns: * A `Package` if one was found, `null` if none exists. */ - private inout(Package) lookup(string name, Version ver, PackageManager mgr) inout { + inout(Package) lookup(string name, Version ver, PackageManager mgr) inout { foreach (pkg; this.localPackages) if (pkg.name == name && pkg.version_.matches(ver, VersionMatchMode.standard)) return pkg; @@ -1391,7 +1406,7 @@ private struct Location { * Returns: * A `Package` if one was found, `null` if none exists. */ - private Package load (string name, Version vers, PackageManager mgr) + Package load (string name, Version vers, PackageManager mgr) { if (auto pkg = this.lookup(name, vers, mgr)) return pkg; @@ -1423,7 +1438,7 @@ private struct Location { * but this function returns `$BASE/$NAME-$VERSION/` * `$BASE` is `this.packagePath`. */ - private NativePath getPackagePath (string name, string vers) + NativePath getPackagePath (string name, string vers) { NativePath result = this.packagePath ~ name ~ vers; result.endsWithSlash = true; diff --git a/source/dub/project.d b/source/dub/project.d index 76cb2e627..875ad6f16 100644 --- a/source/dub/project.d +++ b/source/dub/project.d @@ -1734,8 +1734,8 @@ unittest This is the runtime representation of the information contained in "dub.selections.json" within a package's directory. */ -final class SelectedVersions { - private { +public class SelectedVersions { + protected { enum FileVersion = 1; Selected m_selections; bool m_dirty = false; // has changes since last save diff --git a/source/dub/test/base.d b/source/dub/test/base.d index 4e498762a..efc6478a8 100644 --- a/source/dub/test/base.d +++ b/source/dub/test/base.d @@ -13,10 +13,11 @@ public import std.algorithm; import dub.data.settings; public import dub.dependency; -import dub.dub; -import dub.package_; +public import dub.dub; +public import dub.package_; import dub.packagemanager; import dub.packagesuppliers.packagesupplier; +import dub.project; // TODO: Remove and handle logging the same way we handle other IO import dub.internal.logging; @@ -54,11 +55,163 @@ public class TestDub : Dub return Settings.init; } + /// + protected override PackageManager makePackageManager() const + { + return new TestPackageManager(); + } + /// See `MockPackageSupplier` documentation for this class' implementation protected override PackageSupplier makePackageSupplier(string url) const { return new MockPackageSupplier(url); } + + /// Loads the package from the specified path as the main project package. + public override void loadPackage(NativePath path) + { + assert(0, "Not implemented"); + } + + /// Loads a specific package as the main project package (can be a sub package) + public override void loadPackage(Package pack) + { + m_project = new Project(m_packageManager, pack, new TestSelectedVersions()); + } + + /// Reintroduce parent overloads + public alias loadPackage = Dub.loadPackage; + + /** + * Creates a package with the provided recipe + * + * This is a convenience function provided to create a package based on + * a given recipe. This is to allow test-cases to be written based off + * issues more easily. + * + * In order for the `Package` to be visible to `Dub`, use `addTestPackage`, + * as `makeTestPackage` simply creates the `Package` without adding it. + * + * Params: + * str = The string representation of the `PackageRecipe` + * recipe = The `PackageRecipe` to use + * vers = The version the package is at, e.g. `Version("1.0.0")` + * fmt = The format `str` is in, either JSON or SDL + * + * Returns: + * The created `Package` instance + */ + public Package makeTestPackage(string str, Version vers, PackageFormat fmt = PackageFormat.json) + { + import dub.recipe.io; + final switch (fmt) { + case PackageFormat.json: + auto recipe = parsePackageRecipe(str, "dub.json"); + recipe.version_ = vers.toString(); + return new Package(recipe); + case PackageFormat.sdl: + auto recipe = parsePackageRecipe(str, "dub.sdl"); + recipe.version_ = vers.toString(); + return new Package(recipe); + } + } + + /// Ditto + public Package addTestPackage(string str, Version vers, PackageFormat fmt = PackageFormat.json) + { + return this.packageManager.add(this.makeTestPackage(str, vers, fmt)); + } +} + +/** + * + */ +public class TestSelectedVersions : SelectedVersions { + import dub.recipe.selection; + + /// Forward to parent's constructor + public this(uint version_ = FileVersion) @safe pure nothrow @nogc + { + super(version_); + } + + /// Ditto + public this(Selected data) @safe pure nothrow @nogc + { + super(data); + } + + /// Do not do IO + public override void save(NativePath path) + { + // No-op + } +} + +/** + * A `PackageManager` suitable to be used in unittests + * + * This `PackageManager` does not perform any IO. It imitates the base + * `PackageManager`, exposing 3 locations, but loading of packages is not + * automatic and needs to be done by passing a `Package` instance. + */ +package class TestPackageManager : PackageManager +{ + this() + { + NativePath pkg = NativePath("/tmp/dub-testsuite-nonexistant/packages/"); + NativePath user = NativePath("/tmp/dub-testsuite-nonexistant/user/"); + NativePath system = NativePath("/tmp/dub-testsuite-nonexistant/system/"); + super(pkg, user, system, false); + } + + /// Disabled as semantic are not implementable unless a virtual FS is created + public override @property void customCachePaths(NativePath[] custom_cache_paths) + { + assert(0, "Function not implemented"); + } + + /// Ditto + public override Package store(NativePath src, PlacementLocation dest, string name, Version vers) + { + assert(0, "Function not implemented"); + } + + /** + * This function usually scans the filesystem for packages. + * + * We don't want to do IO access and rely on users adding the packages + * before the test starts instead. + * + * Note: Deprecated `refresh(bool)` does IO, but it's deprecated + */ + public override void refresh() + { + // Do nothing + } + + /** + * Looks up a specific package + * + * Unlike its parent class, no lazy loading is performed. + * Additionally, as they are already deprecated, overrides are + * disabled and not available. + */ + public override Package getPackage(string name, Version vers, bool enable_overrides = false) + { + //assert(!enable_overrides, "Overrides are not implemented for TestPackageManager"); + + // Implementation inspired from `PackageManager.lookup`, + // except we replaced `load` with `lookup`. + if (auto pkg = this.m_internal.lookup(name, vers, this)) + return pkg; + + foreach (ref location; this.m_repositories) + if (auto p = location.lookup(name, vers, this)) + return p; + + return null; + } } /** diff --git a/source/dub/test/dependencies.d b/source/dub/test/dependencies.d new file mode 100644 index 000000000..d9f78ed65 --- /dev/null +++ b/source/dub/test/dependencies.d @@ -0,0 +1,133 @@ +/******************************************************************************* + + Test for dependencies + + This module is mostly concerned with dependency resolutions and visible user + behavior. Tests that check how different recipe would interact with one + another, and how conflicts are resolved or reported, belong here. + + The project (the loaded package) is usually named 'a' and dependencies use + single-letter, increasing name, for simplicity. Version 1.0.0 is used where + versions do not matter. Packages are usually created in reverse dependency + order when possible, unless the creation order matters. + + Test that deal with dependency resolution should not concern themselves with + the registry: instead, packages are added to the `PackageManager`, as that + makes testing the core logic more robust without adding a layer + of complexity brought by the `PackageSupplier`. + + Most tests have 3 parts: First, setup the various packages. Then, run + `dub.upgrade(UpgradeOptions.select)` to create the selection. Finally, + run tests on the resulting state. + +*******************************************************************************/ + +module dub.test.dependencies; + +version (unittest): + +import dub.test.base; + +// Ensure that simple dependencies get resolved correctly +unittest +{ + const a = `name "a" +dependency "b" version="*" +dependency "c" version="*" +`; + const b = `name "b"`; + const c = `name "c"`; + + scope dub = new TestDub(); + dub.addTestPackage(c, Version("1.0.0"), PackageFormat.sdl); + dub.addTestPackage(b, Version("1.0.0"), PackageFormat.sdl); + dub.loadPackage(dub.addTestPackage(a, Version("1.0.0"), PackageFormat.sdl)); + + dub.upgrade(UpgradeOptions.select); + + assert(dub.project.hasAllDependencies(), "project has missing dependencies"); + assert(dub.project.getDependency("b", true), "Missing 'b' dependency"); + assert(dub.project.getDependency("c", true), "Missing 'c' dependency"); + assert(dub.project.getDependency("no", true) is null, "Returned unexpected dependency"); +} + +// Test that indirect dependencies get resolved correctly +unittest +{ + const a = `name "a" +dependency "b" version="*" +`; + const b = `name "b" +dependency "c" version="*" +`; + const c = `name "c"`; + + scope dub = new TestDub(); + dub.addTestPackage(c, Version("1.0.0"), PackageFormat.sdl); + dub.addTestPackage(b, Version("1.0.0"), PackageFormat.sdl); + dub.loadPackage(dub.addTestPackage(a, Version("1.0.0"), PackageFormat.sdl)); + + dub.upgrade(UpgradeOptions.select); + + assert(dub.project.hasAllDependencies(), "project has missing dependencies"); + assert(dub.project.getDependency("b", true), "Missing 'b' dependency"); + assert(dub.project.getDependency("c", true), "Missing 'c' dependency"); + assert(dub.project.getDependency("no", true) is null, "Returned unexpected dependency"); +} + +// Simple diamond dependency +unittest +{ + const a = `name "a" +dependency "b" version="*" +dependency "c" version="*" +`; + const b = `name "b" +dependency "d" version="*" +`; + const c = `name "c" +dependency "d" version="*" +`; + const d = `name "d"`; + + scope dub = new TestDub(); + dub.addTestPackage(d, Version("1.0.0"), PackageFormat.sdl); + dub.addTestPackage(c, Version("1.0.0"), PackageFormat.sdl); + dub.addTestPackage(b, Version("1.0.0"), PackageFormat.sdl); + dub.loadPackage(dub.addTestPackage(a, Version("1.0.0"), PackageFormat.sdl)); + + dub.upgrade(UpgradeOptions.select); + + assert(dub.project.hasAllDependencies(), "project has missing dependencies"); + assert(dub.project.getDependency("b", true), "Missing 'b' dependency"); + assert(dub.project.getDependency("c", true), "Missing 'c' dependency"); + assert(dub.project.getDependency("c", true), "Missing 'd' dependency"); + assert(dub.project.getDependency("no", true) is null, "Returned unexpected dependency"); +} + +// Missing dependencies trigger an error +unittest +{ + const a = `name "a" +dependency "b" version="*" +`; + + scope dub = new TestDub(); + dub.loadPackage(dub.addTestPackage(a, Version("1.0.0"), PackageFormat.sdl)); + + try + dub.upgrade(UpgradeOptions.select); + catch (Exception exc) + assert(exc.message() == `Failed to find any versions for package b, referenced by a 1.0.0`); + + assert(!dub.project.hasAllDependencies(), "project should have missing dependencies"); + assert(dub.project.getDependency("b", true) is null, "Found 'b' dependency"); + assert(dub.project.getDependency("no", true) is null, "Returned unexpected dependency"); + + // Add the missing dependency to our PackageManager + dub.addTestPackage(`name "b"`, Version("1.0.0"), PackageFormat.sdl); + dub.upgrade(UpgradeOptions.select); + assert(dub.project.hasAllDependencies(), "project have missing dependencies"); + assert(dub.project.getDependency("b", true), "Missing 'b' dependency"); + assert(dub.project.getDependency("no", true) is null, "Returned unexpected dependency"); +}