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

Improvement/bb 585 bump deps #2611

Open
wants to merge 8 commits into
base: improvement/BB-615-dependencies-bump
Choose a base branch
from

Conversation

benzekrimaha
Copy link
Contributor

@benzekrimaha benzekrimaha commented Jan 15, 2025

Issue: BB-585

Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.12%. Comparing base (2bfa2b6) to head (ab81bc9).
Report is 1 commits behind head on improvement/BB-615-dependencies-bump.

Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
bin/backbeat.js 48.14% <ø> (ø)
extensions/gc/GarbageCollector.js 40.90% <ø> (ø)
extensions/gc/GarbageCollectorProducer.js 82.60% <ø> (ø)
extensions/gc/service.js 0.00% <ø> (ø)
extensions/index.js 100.00% <ø> (ø)
extensions/ingestion/IngestionQueuePopulator.js 73.01% <ø> (ø)
extensions/ingestion/constants.js 100.00% <ø> (ø)
extensions/lifecycle/LifecycleQueuePopulator.js 56.39% <100.00%> (ø)
...ecycle/bucketProcessor/LifecycleBucketProcessor.js 79.48% <ø> (ø)
extensions/lifecycle/bucketProcessor/task.js 0.00% <ø> (ø)
... and 52 more
Components Coverage Δ
Bucket Notification 75.37% <ø> (ø)
Core Library 75.75% <100.00%> (ø)
Ingestion 69.24% <ø> (ø)
Lifecycle 76.17% <100.00%> (ø)
Oplog Populator 85.06% <100.00%> (ø)
Replication 57.79% <100.00%> (ø)
Bucket Scanner 85.60% <ø> (ø)
@@                          Coverage Diff                          @@
##           improvement/BB-615-dependencies-bump    #2611   +/-   ##
=====================================================================
  Coverage                                 71.12%   71.12%           
=====================================================================
  Files                                       201      201           
  Lines                                     13338    13338           
=====================================================================
  Hits                                       9487     9487           
  Misses                                     3841     3841           
  Partials                                     10       10           
Flag Coverage Δ
api:retry 9.53% <0.00%> (ø)
api:routes 9.34% <0.00%> (ø)
bucket-scanner 85.60% <ø> (ø)
ingestion 12.28% <0.00%> (ø)
lib 7.37% <0.00%> (ø)
lifecycle 18.82% <12.50%> (ø)
notification 1.07% <0.00%> (ø)
replication 18.60% <37.50%> (ø)
unit 48.15% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@benzekrimaha benzekrimaha force-pushed the improvement/BB-585-bump-deps branch 2 times, most recently from c55e193 to 9b15fe7 Compare January 16, 2025 08:14
@benzekrimaha benzekrimaha force-pushed the improvement/BB-585-bump-deps branch from 9b15fe7 to ab81bc9 Compare January 16, 2025 08:16
@benzekrimaha benzekrimaha marked this pull request as ready for review January 16, 2025 08:16
@@ -26,6 +26,14 @@ jobs:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ github.token }}

- name: Install Oras
Copy link
Contributor

Choose a reason for hiding this comment

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

fix already merged

@@ -196,7 +196,7 @@ jobs:
token: ${{ secrets.CODECOV_TOKEN }}

- name: run ballooning tests for lifecycle conductor
run: yarn mocha tests/performance/lifecycle/conductor-check-memory-balloon.js
run: yarn mocha tests/performance/lifecycle/conductor-check-memory-balloon.js --exit
Copy link
Contributor

Choose a reason for hiding this comment

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

the --exit flag is not recomended by mocha :

TL;DR: If your tests hang after an upgrade to Mocha v4.0.0 or newer, use --exit for a quick (though not necessarily recommended) fix.

Prior to version v4.0.0, by default, Mocha would force its own process to exit once it was finished executing all tests. This behavior enables a set of potential problems; it’s indicative of tests (or fixtures, harnesses, code under test, etc.) which don’t clean up after themselves properly. Ultimately, “dirty” tests can (but not always) lead to false positive or false negative results.

“Hanging” most often manifests itself if a server is still listening on a port, or a socket is still open, etc. It can also be something like a runaway setInterval(), or even an errant Promise that never fulfilled.

The default behavior in v4.0.0 (and newer) is --no-exit, where previously it was --exit.

The easiest way to “fix” the issue is to pass --exit to the Mocha process. It can be time-consuming to debug — because it’s not always obvious where the problem is — but it is recommended to do so.

  • why do we need to add it?
  • if we don't know why (and don't have the time to debug or prefer to merge this 'large' PR first to avoid more conflicts/...), then we must create a ticket already so we do a better fix ASAP.

@@ -1,6 +1,7 @@
# Zenko Backbeat

![backbeat logo](res/backbeat-logo.png)
[![Build Status][badgepriv]][badgepub]
Copy link
Contributor

Choose a reason for hiding this comment

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

these badge are not correct, the URLs (at the end of this file) should be updated

@@ -17,17 +17,17 @@
"oplog_populator": "node extensions/oplogPopulator/OplogPopulatorTask.js",
"mongo_queue_processor": "node extensions/mongoProcessor/mongoProcessorTask.js",
"garbage_collector": "node extensions/gc/service.js",
"test": "mocha --recursive tests/unit --timeout 30000",
"test": "mocha --recursive tests/unit --timeout 30000 --exit",
Copy link
Contributor

Choose a reason for hiding this comment

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

same, it is recommneded not to use the flag, and fix the test(s) that need it...

Comment on lines +166 to 168
if (a.VersionId < b.VersionId) {return -1;}
if (a.VersionId > b.VersionId) {return 1;}
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (a.VersionId < b.VersionId) {return -1;}
if (a.VersionId > b.VersionId) {return 1;}
return 0;
if (a.VersionId < b.VersionId) {
return -1;
}
if (a.VersionId > b.VersionId) {
return 1;
}
return 0;

@@ -1,4 +1,4 @@
'use strict'; // eslint-disable-line
'use strict';
Copy link
Contributor

@francoisferrand francoisferrand Jan 31, 2025

Choose a reason for hiding this comment

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

extra spaces at end of line (in many files actually)

Suggested change
'use strict';
'use strict';

@@ -1,4 +1,4 @@
'use strict'; // eslint-disable-line
'use strict';
Copy link
Contributor

Choose a reason for hiding this comment

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

extra spaces at end of line

@@ -19,14 +19,13 @@ jobs:
- name: Checkout
uses: actions/checkout@v4

# TODO: remove the following step once Oras CLI 0.13.0 bug https://github.com/oras-project/oras/issues/447 is fixed.
- name: Downgrade Oras to 0.12.0
- name: Install Oras
Copy link
Contributor

Choose a reason for hiding this comment

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

fix already merged

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.

2 participants