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

Implement true integration tests - Find & Resolve bugs discovered due to it #238

Merged
merged 5 commits into from
Feb 3, 2024

Conversation

confused-Techie
Copy link
Member

Requirements

  • Filling out the template is required.

  • All new code requires tests to ensure against regressions.

    • However, if your PR contains zero code changes, feel free to select the checkmark below to indicate so.
  • Have you ran tests against this code?

  • This PR contains zero code changes.

Description of the Change

Originally this PR aimed to implement nock to achieve true 100% integration tests. By simple mocking the external endpoints we had to hit, setting up a database, and hitting our own backend to see what it would do.

A valiant goal in and of itself, but in doing so, I was able to go ahead and resolve one known bug, another unknown bug, as well as improve the user experience considerably.

  • [Known Bug] Via a new constructNewPackagePublishData builder (much like the builders we already have) I was able to retire the PackageObject builder as it was clunky, and was the cause of bugs itself. Such that it would incorrectly create versioned data during package publication. Resulting in the incorrect tag tarball being selected for the latest tag tarball. This would mean that while the version number was correct, the tarball would point to the wrong location, causing bad installations.

Fixes #236

  • [Unknown] On every package publish the backend would try to send out a webhook to Discord. But if the publish failed, then the webhook would crash, causing the server to crash as a whole. This has apparently been happening since we implemented webhooks way back when.
  • [Improved Experience] The new package data builder does actually some minimal work to validate the data it's going to publish, there still is more that can be done here, but as of now it will validate version information, meaning that if it can't find the correct version info it will return a detailed error to the user, which previously it'd report just "Server Error". Additionally, this new builder has many more safeguards in place to discover required info, such as the owner key which will search the package.json name field then name of the provided URL for publication, then the parsed URL of the repository key (supporting both string and object form) and finally grabbing the author field. With this being easily expandable in the future for other values.

@confused-Techie confused-Techie merged commit d7e4553 into main Feb 3, 2024
6 checks passed
@confused-Techie confused-Techie deleted the full-tests branch February 3, 2024 07:57
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.

[BUG] Package Published linking to non-latest tarball via GitHub
1 participant