Conversation
G-Rath
left a comment
There was a problem hiding this comment.
Looks good, just got a few changes - would you mind also adding an "empty.lock" (which is "no packages", not "completely empty file") and "two-packages.lock" fixtures as well? (I know technically your other tests cover this, but it's consistent with the standard set of fixtures we have for our other lockfiles)
There was a problem hiding this comment.
would you mind restructuring these to use individual functions to be consistent with the rest of our lockfile tests
pkg/lockfile/parse-pdm-lock.go
Outdated
| const PdmLockFileName = "pdm.lock" | ||
| const pdmGroupDefault = "default" | ||
| const pdmGroupDev = "dev" |
There was a problem hiding this comment.
we don't typically use constants for these, and you're only referencing them each once so lets just inline them
| type PdmLockPackage struct { | ||
| Name string `toml:"name"` | ||
| Version string `toml:"version"` | ||
| Groups []string `toml:"groups"` |
There was a problem hiding this comment.
just noting for others: it looks like pdm only very recently started recording dependency groups at the package level in its lockfiles - I couldn't actually find any locks searching GitHub, but the PRs on pdm support this feature as being added
| } | ||
|
|
||
| //nolint:gochecknoinits |
There was a problem hiding this comment.
we should ensure the extractor matches the interface:
| } | |
| //nolint:gochecknoinits | |
| } | |
| var _ Extractor = PdmLockExtractor{} | |
| //nolint:gochecknoinits |
pkg/lockfile/parse-pdm-lock.go
Outdated
| const pdmGroupDefault = "default" | ||
| const pdmGroupDev = "dev" | ||
|
|
||
| type PdmExtractor struct{} |
There was a problem hiding this comment.
This matches the naming we use for the other extractors:
| type PdmExtractor struct{} | |
| type PdmLockExtractor struct{} |
Added requested tests. Amended fixes to the existing commit. |
G-Rath
left a comment
There was a problem hiding this comment.
Awesome stuff! It'd be great if you could space out the tests a bit, but I think this is good to go!
| } | ||
|
|
||
| func TestParsePdmLock_FileDoesNotExist(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
nit: these are pretty squashed together
usually I like to have a new line between every function, after t.Parallel(), and before the first expect* call
There was a problem hiding this comment.
Added newlines to test functions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #776 +/- ##
==========================================
+ Coverage 79.85% 80.02% +0.17%
==========================================
Files 90 91 +1
Lines 6174 6213 +39
==========================================
+ Hits 4930 4972 +42
+ Misses 1040 1038 -2
+ Partials 204 203 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds support for parsing package version information from lockfiles of pdm, package and dependency manager for Python.
Add support for parsing package information from
pdm.lock-files used bypdm, package and dependency manager for Python (https://pdm-project.org/latest/)