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

Uninstall adapter from package-lock.json only before re-installing tarball #613

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

MiSchroe
Copy link
Contributor

Fixes #612

Copy link
Collaborator

@AlCalzone AlCalzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid this is also not working as intended. According to https://docs.npmjs.com/cli/v10/commands/npm-update, this will update all installed packages to their latest version, including js-controller, admin and the adapter's dependencies.

I think the best solution for now is to rename the tarball to something random, so npm always installs it

@MiSchroe
Copy link
Contributor Author

@AlCalzone Renaming the tarball wouldn't help. npm install will lookup the package by its name not by the filename. Then it checks against the package-lock.json (and there is another one in the node_modules folder) where it will find it. And after finding a reference in the package-lock.json it takes the checksum from there, finds the package based on the old checksum in the cache and installs it from there.

I have found another solution:

  1. npm pack (like it is implemented already)
  2. Run npm uninstall <adapter> --package-lock-only -> Deletes the entries from the package-lock.json files without removing files from the node_modules folder.
  3. Install the adapter as it implemented right now.

The advantage:

  • No manual fiddling in package-lock.json files.
  • No deletion of dependencies.
  • No unwanted updates of dependencies

If this is a feasible solution I can change this pull request accordingly.

@mcm1957
Copy link
Contributor

mcm1957 commented Jul 25, 2024

Why using --pachage-lock-only? The adapter will be installed afterwards anyway. And removing any rest of the adapter can only improve testing.

@MiSchroe
Copy link
Contributor Author

Why using --pachage-lock-only? The adapter will be installed afterwards anyway. And removing any rest of the adapter can only improve testing.

That would uninstall the dependencies, too.

@mcm1957
Copy link
Contributor

mcm1957 commented Jul 25, 2024

Ok would cause some time. But would ensure to install deps exactly as listed in package.json. if changed.Pro and contra.

But yes could be too much overhead for major Situations

@AlCalzone
Copy link
Collaborator

@MiSchroe I think it is worth a shot

…move the adapter from the package-lock.json file(s).
@MiSchroe MiSchroe requested a review from AlCalzone July 29, 2024 08:01
@AlCalzone AlCalzone changed the title Fixes Subsequent integration tests won't grab the newest source code but will re-use a cached version #612 Uninstall adapter from package-lock.json only before re-installing tarball Jul 29, 2024
Copy link
Collaborator

@AlCalzone AlCalzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor thing. I assume that you've tested this locally?

src/tests/integration/lib/adapterSetup.ts Show resolved Hide resolved
@MiSchroe MiSchroe requested a review from AlCalzone July 30, 2024 14:15
@AlCalzone AlCalzone merged commit 2aaf00d into ioBroker:master Jul 30, 2024
7 checks passed
@MiSchroe MiSchroe deleted the MiSchroe/issue612 branch July 31, 2024 13:09
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.

Subsequent integration tests won't grab the newest source code but will re-use a cached version
3 participants