-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/refactor package name class #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1c5fd27
ca9db8e
34e7771
3fc79c9
41286d5
0fed7d9
530f45c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "python.testing.pytestArgs": [ | ||
| "src" | ||
| ], | ||
| "python.testing.unittestEnabled": false, | ||
| "python.testing.pytestEnabled": true | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,18 @@ | ||
| certifi==2023.5.7 | ||
| charset-normalizer==3.1.0 | ||
| colorama==0.4.6 | ||
| exceptiongroup==1.2.0 | ||
| idna==3.4 | ||
| iniconfig==2.0.0 | ||
| inquirerpy==0.3.4 | ||
| ordered-set==4.1.0 | ||
| packaging==23.2 | ||
| pfzy==0.3.4 | ||
| pluggy==1.3.0 | ||
| prompt-toolkit==3.0.38 | ||
| pytest==7.4.3 | ||
| requests==2.30.0 | ||
| termcolor==2.3.0 | ||
| tomli==2.0.1 | ||
| urllib3==2.0.2 | ||
| wcwidth==0.2.6 |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,8 @@ | |||||
| import time | ||||||
| import requests | ||||||
|
|
||||||
| import Package | ||||||
|
|
||||||
| version_file = os.path.join(os.path.dirname(sys.argv[0]), 'version.json') | ||||||
| with open(version_file, "r") as f: | ||||||
| data = json.load(f) | ||||||
|
|
@@ -63,22 +65,21 @@ def colored(text, color): | |||||
| def cprint(text, color): | ||||||
| print(text) | ||||||
|
|
||||||
| # Prepend the @loupeteam prefix to all package names, and split any @version suffixes off into separate struct. | ||||||
| packages = [] | ||||||
| packageVersions = [] | ||||||
| # Build array of PackageNameVersion objects from args | ||||||
| packageNameObjectList = [] | ||||||
| if(args.packages): | ||||||
| for item in args.packages: | ||||||
| # Force package name to lowercase, by convention | ||||||
| item = item.lower() | ||||||
| # If the @ character is present, it means there's a version specifier. | ||||||
| if('@' in item): | ||||||
| splitItem = item.split('@') | ||||||
| packages.append('@loupeteam/' + splitItem[0]) | ||||||
| packageVersions.append(splitItem[1]) | ||||||
| # Othersie, version string is empty. | ||||||
| else: | ||||||
| packages.append('@loupeteam/' + item) | ||||||
| packageVersions.append('') | ||||||
| try: | ||||||
| packageNameObjectList.append(Package.PackageNameVersion(item)) | ||||||
| except: | ||||||
| print(colored(f'Could not interpret \"{item}\" as a valid package', 'red')) | ||||||
|
|
||||||
| # Return if no valid packages | ||||||
| if len(packageNameObjectList) == 0: | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
is more pythonic. |
||||||
| return | ||||||
| packagesFullNames = [p.fullName for p in packageNameObjectList] | ||||||
| packagesBaseNames = [p.baseName for p in packageNameObjectList] | ||||||
| packagesVersions = [p.version for p in packageNameObjectList] | ||||||
|
Comment on lines
+80
to
+82
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't get why have you implemented |
||||||
|
|
||||||
| # Authenticate with a custom personal access token. | ||||||
| if(args.cmd == 'login'): | ||||||
|
|
@@ -140,11 +141,13 @@ def cprint(text, color): | |||||
| # View information about a package. | ||||||
| elif(args.cmd == 'view') | (args.cmd == 'info'): | ||||||
| if (len(args.packages) > 1): | ||||||
| packageFullName = Package.PackageNameVersion(args.packages[0]).fullName | ||||||
| options = args.packages[1:] | ||||||
| getInfo(packages[0], options) | ||||||
| getInfo(packageFullName, options) | ||||||
| elif (len(args.packages) > 0): | ||||||
| packageFullName = Package.PackageNameVersion(args.packages[0]).fullName | ||||||
| options = [] | ||||||
| getInfo(packages[0], options) | ||||||
| getInfo(packageFullName, options) | ||||||
| else: | ||||||
| print(colored('Please provide the name of one package.', 'yellow')) | ||||||
|
|
||||||
|
|
@@ -154,16 +157,16 @@ def cprint(text, color): | |||||
|
|
||||||
| # Retrieve the type of a package (or directory). | ||||||
| elif(args.cmd == 'type'): | ||||||
| if (len(packages) > 1): | ||||||
| if (len(packagesFullNames) > 1): | ||||||
| print(colored('This command is only supported with a single package.', 'yellow')) | ||||||
| else: | ||||||
| print(getPackageType(args.packages[0])) | ||||||
|
|
||||||
| # Open up documentation page for the library. | ||||||
| elif(args.cmd == 'docs'): | ||||||
| if (len(packages) > 0): | ||||||
| print('Opening up documentation for ' + ', '.join(packages) + '...') | ||||||
| openDocumentation(packages) | ||||||
| if (len(packagesFullNames) > 0): | ||||||
| print('Opening up documentation for ' + ', '.join(packagesFullNames) + '...') | ||||||
| openDocumentation(packagesFullNames) | ||||||
| print(colored('Documentation is now up in your browser.', 'green')) | ||||||
| else: | ||||||
| print(colored('Please provide the name of at least one package.', 'yellow')) | ||||||
|
|
@@ -267,25 +270,25 @@ def cprint(text, color): | |||||
| elif(args.cmd == 'install'): | ||||||
| # Handle default scenario where sources are not requested (i.e. we're installing binary packages). | ||||||
| if (not args.source): | ||||||
| if (len(packages) > 0): | ||||||
| print('Installing ' + ', '.join(packages) + '...') | ||||||
| if (len(packagesFullNames) > 0): | ||||||
| print('Installing ' + ', '.join(packagesFullNames) + '...') | ||||||
| try: | ||||||
| installPackages(packages, packageVersions) | ||||||
| installPackages(packagesFullNames, packagesVersions) | ||||||
| except: | ||||||
| cprint('Error while attempting to install package(s).', 'yellow') | ||||||
| return | ||||||
| else: | ||||||
| print('Installing all dependencies...') | ||||||
| try: | ||||||
| installPackages(packages, packageVersions) | ||||||
| installPackages(packagesFullNames, packagesVersions) | ||||||
| except: | ||||||
| cprint('Error while attempting to install package(s).', 'yellow') | ||||||
| return | ||||||
| # Move packages from the node_modules folder into the project/main directory. | ||||||
| syncPackages(getAllDependencies(packages)) | ||||||
| syncPackages(getAllDependencies(packagesFullNames)) | ||||||
| else: # Handle request to install source code instead. | ||||||
| if (len(packages) > 0): | ||||||
| print('Cloning ' + ', '.join(args.packages) + '...') | ||||||
| if (len(packageNameObjectList) > 0): | ||||||
| print('Cloning ' + ', '.join(packagesBaseNames) + '...') | ||||||
| try: | ||||||
| # First check to see if there is an AS project locally. | ||||||
| project = ASTools.Project('.') | ||||||
|
|
@@ -297,19 +300,10 @@ def cprint(text, color): | |||||
| print('Loupe folder not found, creating it...') | ||||||
| # Add the Loupe package back in there. | ||||||
| loupePkg = librariesPkg.addEmptyPackage('Loupe') | ||||||
| for package in args.packages: | ||||||
| # Extract version info if included (this would show up after the '@' character, i.e. 'mylib@3.0.4') | ||||||
| # TODO: this check is now redundant with a version check done at the top of this file, so this should get refactored... | ||||||
| if package.find('@') > -1: | ||||||
| splitPackage = package.split('@') | ||||||
| packageName = splitPackage[0] | ||||||
| packageVersion = splitPackage[1] | ||||||
| installSource(packageName, packageVersion, loupePkg) | ||||||
| else: | ||||||
| packageName = package | ||||||
| installSource(packageName, '', loupePkg) | ||||||
| for (packageFullName, packageBaseName, version) in zip(packagesFullNames, packagesBaseNames, packagesVersions): | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you zip them together to iterate over three lists but if you would have kept it in a single list it would be as easy as: for package in packages:
installSource(package.fullName, package.version, loupePkg)or with further refactoring to for package in packages:
installSource(package) |
||||||
| installSource(packageFullName, version, loupePkg) | ||||||
| # Next, install any dependencies of this source library. | ||||||
| sourceDependencies = getSourceDependencies(os.path.join('.', 'Logical', 'Libraries', 'Loupe', packageName)) | ||||||
| sourceDependencies = getSourceDependencies(os.path.join('.', 'Logical', 'Libraries', 'Loupe', packageBaseName)) | ||||||
| if (len(sourceDependencies) > 0): | ||||||
| # TODO: add support for getting the correct version of these dependencies. | ||||||
| installPackages(sourceDependencies, [''] * len(sourceDependencies)) | ||||||
|
|
@@ -322,18 +316,18 @@ def cprint(text, color): | |||||
| # First check project-level settings to see if deployment is configured. | ||||||
| deploymentConfigs = getPackageManifestField('package.json', ['lpmConfig', 'deploymentConfigs']) | ||||||
| if (deploymentConfigs is not None): | ||||||
| print('Deploying ' + ', '.join(packages) + ' to the following configurations: ' + ', '.join(deploymentConfigs)) | ||||||
| print('Deploying ' + ', '.join(packagesFullNames) + ' to the following configurations: ' + ', '.join(deploymentConfigs)) | ||||||
| for config in deploymentConfigs: | ||||||
| if(not args.source): | ||||||
| deployPackages(config, getAllDependencies(packages)) | ||||||
| deployPackages(config, getAllDependencies(packagesFullNames)) | ||||||
| else: | ||||||
| # TODO: this split below may not be necessary, TBD. | ||||||
| # For case of source, deploy the source first. | ||||||
| deployPackages(config, packages) | ||||||
| deployPackages(config, packagesFullNames) | ||||||
| # Then deploy all of its dependencies. | ||||||
| deployPackages(config, sourceDependencies) | ||||||
| cprint('Operation completed successfully.', 'green') | ||||||
| for package in packages: | ||||||
| for package in packagesFullNames: | ||||||
| packageManifestPath = os.path.join('node_modules', package, 'package.json') | ||||||
| if(os.path.exists(packageManifestPath)): | ||||||
| packageType = getPackageManifestField(packageManifestPath, ['lpm', 'type']) | ||||||
|
|
@@ -345,14 +339,14 @@ def cprint(text, color): | |||||
|
|
||||||
| # Uninstall one or more packages. | ||||||
| elif(args.cmd == 'uninstall'): | ||||||
| if (len(packages) > 0): | ||||||
| print('Uninstalling ' + ', '.join(packages) + '...') | ||||||
| if (len(packagesFullNames) > 0): | ||||||
| print('Uninstalling ' + ', '.join(packagesFullNames) + '...') | ||||||
| try: | ||||||
| uninstallPackages(packages) | ||||||
| uninstallPackages(packagesFullNames) | ||||||
| except: | ||||||
| cprint('Error while attempting to uninstall package(s).', 'yellow') | ||||||
| return | ||||||
| syncPackages(getAllDependencies(packages)) | ||||||
| syncPackages(getAllDependencies(packagesFullNames)) | ||||||
| cprint('Operation completed successfully.', 'green') | ||||||
| else: | ||||||
| print(colored('Please provide the name of at least one package.', 'yellow')) | ||||||
|
|
@@ -364,13 +358,13 @@ def cprint(text, color): | |||||
| if(gitClient == ''): | ||||||
| cprint("No Git client configured (please run lpm configure)", "yellow") | ||||||
| else: | ||||||
| print(f'Opening {gitClient} for these packages: ' + ', '.join(args.packages)) | ||||||
| for package in packages: | ||||||
| print(f'Opening {gitClient} for these packages: ' + ', '.join(packagesFullNames)) | ||||||
| for package in packagesBaseNames: | ||||||
| cmd = [] | ||||||
| if(gitClient == 'GitExtensions'): | ||||||
| cmd.append('gitex.cmd') | ||||||
| cmd.append('openrepo') | ||||||
| cmd.append('\"' + os.path.join(os.getcwd(), 'Logical', 'Libraries', 'Loupe', os.path.split(package)[1]) + '\"') | ||||||
| cmd.append('\"' + os.path.join(os.getcwd(), 'Logical', 'Libraries', 'Loupe', package) + '\"') | ||||||
| executeAndContinue(cmd) | ||||||
| else: | ||||||
| cprint(f"We don't support {gitClient}, are you kidding?", "yellow") | ||||||
|
|
@@ -417,7 +411,7 @@ def cprint(text, color): | |||||
|
|
||||||
| # In all other cases, just pass the command and list of packages straight through to NPM. | ||||||
| else: | ||||||
| runGenericNpmCmd(args.cmd, packages) | ||||||
| runGenericNpmCmd(args.cmd, args.packages) | ||||||
| cprint('Operation completed', 'green') | ||||||
|
|
||||||
| return | ||||||
|
|
@@ -477,7 +471,8 @@ def importLibraries(): | |||||
| packages_to_import = [] | ||||||
| package_versions_to_import = [] | ||||||
| for library in loupePkg.objects: | ||||||
| packages_to_import.append('@loupeteam/' + library.text) | ||||||
| packageFullName = Package.PackageNameVersion(library.text).fullName # Adds organization prefix | ||||||
| packages_to_import.append(packageFullName) | ||||||
| package_versions_to_import.append(library.version) | ||||||
| # Install them. | ||||||
| try: | ||||||
|
|
@@ -615,12 +610,12 @@ def getPackageType(path): | |||||
| return packageType | ||||||
|
|
||||||
| # Perform the NPM install. | ||||||
| def installPackages(packages, packageVersions): | ||||||
| def installPackages(packages, packagesVersions): | ||||||
| command = [] | ||||||
| command.append('npm install') | ||||||
| # force packages names to lowercase | ||||||
| packages = [package.lower() for package in packages] | ||||||
| for (item, version) in zip(packages, packageVersions): | ||||||
| for (item, version) in zip(packages, packagesVersions): | ||||||
| if(version != ''): | ||||||
| command.append(f'{item}@{version}') | ||||||
| else: | ||||||
|
|
@@ -720,9 +715,8 @@ def getAllDependencies(packages): | |||||
| # If there is no package.json for this package, assume it's a source library, and that it's already sync'd | ||||||
| # to the Logical View under Libraries / Loupe. | ||||||
| else: | ||||||
| # Strip it of its @loupeteam prefix. | ||||||
| splitPackage = os.path.split(package)[1] | ||||||
| dependencyData = getSourceDependencies(os.path.join('.', 'Logical', 'Libraries', 'Loupe', splitPackage)) | ||||||
| packageBaseName = Package.PackageNameVersion(package).baseName | ||||||
| dependencyData = getSourceDependencies(os.path.join('.', 'Logical', 'Libraries', 'Loupe', packageBaseName)) | ||||||
| print('Source dependencies: ') | ||||||
| print(dependencyData) | ||||||
| localDependencies = [] | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||||||||
| import re | ||||||||||||
|
|
||||||||||||
| class PackageNameVersion: | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't |
||||||||||||
| def __init__(self, input: str): | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you've already started typing your code:
Suggested change
|
||||||||||||
| reValidInput = r"^(@loupeteam[\\\/])?(\w+)@?(@v?(\d+\.\d+\.\d+))?$" | ||||||||||||
| match = re.search(reValidInput, input, re.IGNORECASE) | ||||||||||||
| if not match: raise ValueError | ||||||||||||
|
Comment on lines
+6
to
+7
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A walrus operator could be used here:
Suggested change
Still two lines of code but reads nicely. |
||||||||||||
| self._baseName = match.group(2).lower() | ||||||||||||
| self._versionText = match.group(4) if match.group(4) is not None else '' | ||||||||||||
|
|
||||||||||||
| @property | ||||||||||||
| def baseName(self): | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| return f"{self._baseName}" | ||||||||||||
|
|
||||||||||||
| @property | ||||||||||||
| def fullName(self): | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| return f"@loupeteam/{self._baseName}" | ||||||||||||
|
|
||||||||||||
| @property | ||||||||||||
| def version(self): | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| if self._versionText != '': | ||||||||||||
| return self._versionText | ||||||||||||
| else: | ||||||||||||
| return '' | ||||||||||||
|
Comment on lines
+21
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it the same?
Suggested change
What was your intention behind this check? |
||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import pytest | ||
| import sys | ||
| sys.path.append('./src/') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Manipulation of path smells bad. What you can do instead is restructure your code to the following: This way you could write in your test file from lpm import packageAnd run tests from the root of the repo. Extracting |
||
| from Package import PackageNameVersion | ||
|
|
||
| class Test_PackageNameVersion: | ||
| def test_validInputsNoVersion(self): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that you use table driven tests but I have a question: why haven't you used pytest parametrisation feature? |
||
|
|
||
| # valid inputs without version number | ||
| validInputsNoVersion = ["atn", | ||
| "atn@", | ||
| "@loupeteam/atn", | ||
| "@loupeteam\\atn", | ||
| "@loupeteam/atn@", | ||
| "@loupeteam\\atn@", | ||
| "@LOUPETEAM/atn", | ||
| "@lOuPeTeAm/atn", | ||
| "AtN", | ||
| ] | ||
|
|
||
| for i in validInputsNoVersion: | ||
| p = PackageNameVersion(i) | ||
| assert p.baseName == "atn" | ||
| assert p.fullName == "@loupeteam/atn" | ||
| assert p.version == "" | ||
|
|
||
| def test_validInputsWithVersion(self): | ||
| # Valid inputs with version number | ||
| validInputsWithVersion = [ "atn@v3.1.0", | ||
| "atn@V3.1.0", | ||
| "atn@3.1.0", | ||
| "@loupeteam/atn@v3.1.0", | ||
| "@loupeteam/atn@V3.1.0", | ||
| "@loupeteam/atn@3.1.0", | ||
| "@loupeteam\\atn@v3.1.0", | ||
| "@loupeteam\\atn@V3.1.0", | ||
| "@loupeteam\\atn@3.1.0", | ||
| ] | ||
|
|
||
| for i in validInputsWithVersion: | ||
| p = PackageNameVersion(i) | ||
| assert p.baseName == "atn" | ||
| assert p.fullName == "@loupeteam/atn" | ||
| assert p.version == "3.1.0" | ||
|
|
||
| def test_versionLeadingZeros(self): | ||
| # Leading zeros are not truncated | ||
| # (though its debatable whether we want to parse to integer) | ||
| p = PackageNameVersion("atn@v03.01.00") | ||
| assert p.baseName == "atn" | ||
| assert p.fullName == "@loupeteam/atn" | ||
| assert p.version == "03.01.00" | ||
|
|
||
| def test_invalidInputs(self): | ||
| # Invalid inputs | ||
| invalidInputs = ["atn@v3.1", | ||
| "atn@v3.1.0.1", | ||
| "atn@w3.1.0", | ||
| "atn@v3.a.0", | ||
| "@loupeteamatn", | ||
| "@somebogusorg/atn", | ||
| "somebogusorg/atn", | ||
| "loupeteam/atn", # maybe this should be valid | ||
| "#~atn$%~" | ||
| ] | ||
|
|
||
| for i in invalidInputs: | ||
| with pytest.raises(ValueError): | ||
| p = PackageNameVersion(i) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could split this file into
requirements.txtthat contains only dependencies that are required to run LPM andrequirements-dev.txtthat contain dependencies to develop LPM.If I'm not mistaken you could even include one into another by adding this to the dev file: