Skip to content
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

Change database.test.js to better test package renaming #269

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

savetheclocktower
Copy link

Tests for package renaming scenarios.

The main difference is that these use insertNewPackageVersion instead of insertNewPackageName. The latter is no longer really used; insertNewPackageVersion also better captures the fact that a renamed package necessarily has at least two versions.

I'd changed getPackageByName.js locally in such a way as to make these tests pass, but my approach locally won't actually work; the database migration proposed by @confused-Techie is wiser. My way of figuring out a package's canonical name was to find out what the name was on the most recently published version of the package, but that approach won't work if a package is renamed and then the version associated with the renaming is deleted. In that scenario, we need a way of figuring out that the newer package name is still canonical even though there aren't any (non-deleted) package versions under that name.

@savetheclocktower
Copy link
Author

It occurred to me just now that these tests pass on master. Since the package backend doesn't seem to do the right thing at the moment, these tests shouldn't pass. I'll dig deeper.

@savetheclocktower savetheclocktower marked this pull request as draft August 4, 2024 21:21
@savetheclocktower
Copy link
Author

Yeah, this isn't the problem — the main problem is in ppm. There's a problem somewhere in this repo, since I got an internal server error when I fixed the bug in ppm, but we can figure that out later.

These tests are still more accurate, but they're not urgent to land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant