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

Fix matching logic for logs from namespace when lines = 0 #5660

Merged
merged 1 commit into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/API/Log.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,11 @@ Log.stream = function(Client, id, raw, timestamp, exclusive, highlight) {
var min_padding = 3

bus.on('log:*', function(type, packet) {
if (id !== 'all'
&& packet.process.name != id
&& packet.process.pm_id != id)
var isMatchingProcess = id === 'all'
|| packet.process.name === id
|| packet.process.pm_id === id
|| packet.process.namespace === id;
if (!isMatchingProcess)
Comment on lines +101 to +105
Copy link
Contributor Author

Choose a reason for hiding this comment

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

separating out isMatchingProcess felt more readable to me - happy to revert back to something like this instead:

        if (id !== 'all'
          && packet.process.name != id
          && packet.process.pm_id != id
          && packet.process.namespace != id)

return;

if ((type === 'out' && exclusive === 'err')
Expand Down
1 change: 1 addition & 0 deletions test/e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ runTest ./test/e2e/logs/log-entire.sh
runTest ./test/e2e/logs/log-null.sh
runTest ./test/e2e/logs/log-json.sh
runTest ./test/e2e/logs/log-create-not-exist-dir.sh
runTest ./test/e2e/logs/log-namespace.sh

# MODULES
runTest ./test/e2e/modules/get-set.sh
Expand Down
35 changes: 35 additions & 0 deletions test/e2e/logs/log-namespace.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the most recently added test/e2e/logs/ case as a template, hoping to use the most modern test patterns

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/usr/bin/env bash

SRC=$(cd $(dirname "$0"); pwd)
source "${SRC}/../include.sh"

cd $file_path/log-namespace/

LOG_PATH_PREFIX="${SRC}/__log-namespace__"

rm -rf "${LOG_PATH_PREFIX}"
mkdir "${LOG_PATH_PREFIX}"

$pm2 start echo.js --namespace e2e-test-log-namespace

LOG_FILE_BASELINE="${LOG_PATH_PREFIX}/baseline-out.log"
$pm2 logs e2e-test-log-namespace > $LOG_FILE_BASELINE & # backgrounded - will be stopped by `$pm2 delete all`
Copy link
Contributor Author

@bawjensen bawjensen Aug 19, 2023

Choose a reason for hiding this comment

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

I haven't seen any e2e (or programmatic tests) for the pm2 logs command, in order to mimic those. So this is a bit of a shot in the dark, with this approach of backgrounding the process.


sleep 2 # should leave time for ~40 "tick" lines

# Using -q to avoid spamming, since there will be a fair few "tick" matches
grep -q "tick" ${LOG_FILE_BASELINE}
spec "Should have 'tick' in the log file"

LOG_FILE_LINES_ZERO="${LOG_PATH_PREFIX}/lines-zero-out.log"
$pm2 logs e2e-test-log-namespace --lines 0 > $LOG_FILE_LINES_ZERO &

sleep 2 # should leave time for ~40 "tick" lines

# Using -q to avoid spamming, since there will be a fair few "tick" matches
grep -q "tick" ${LOG_FILE_LINES_ZERO}
spec "Should have 'tick' in the log file even if using --lines 0"

cd ${SRC}
rm -rf "${LOG_PATH_PREFIX}"
$pm2 delete all
4 changes: 4 additions & 0 deletions test/fixtures/log-namespace/echo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
console.log("start");
setInterval(function () {
console.log("tick");
}, 50);