Skip to content

Conversation

mshanemc
Copy link
Contributor

@mshanemc mshanemc commented Jun 13, 2024

What does this PR do?

  • there were 2 ways of colorizing apex logs (1 for get, 1 for tail)
  • let's just have one that combines the behavior of both
  • there was an env that would let you "bring your own color scheme" and we imported several dependencies to support that. Nobody was using it, per telemetry, so dropped that feature.

general cleanup

  • consolidate duplicate flags
  • prefer sf-plugins-core's StandardColors definitions over custom
  • chalk => ansis

What issues does this PR fix or reference?

@W-15999434@


testing notes

I tested this using dreamhouse.
you can tail log an org and it'll set the log capture stuff. Open the PropertyExplorer to start seeing logs.
once you've done that, you can also list log and get log from the generated logs

@mshanemc mshanemc requested a review from a team as a code owner June 13, 2024 18:34
}
}

const formatForTable = (logRecord: LogRecord): LogForTable => ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's really only 2 fields that needed transorming. The rest can use the name the server uses

return [];
}

logRecords.map((logRecord) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was doing mutation on the records. You'll see UT that were relying on that side effect to pass.


No debug logs found in org

# appColHeader
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a message for table headers seemed silly--we don't normally do that

]);
});

it('will list multiple logs --json', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wasn't actually testing --json, though asserting the result is kinda like it.

expect(result).to.deep.equal(logRecords);
sandbox.stub(LogService.prototype, 'getLogRecords').resolves(structuredClone(logRecords));
await new Log([], config).run();
expect(tableStub.firstCall.args[0]).to.deep.equal([
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to change the test because the implementation doesn't make all the new columns anymore.

doesn't assert the output structure since the next test does that

@cristiand391
Copy link
Member

cristiand391 commented Jun 18, 2024

had to push some changes to fix NUTs/merge conflicts, see last 2 commits

QA notes:

apex tail log --color matches apex get log colors
apex list log lowercase table header, no JSON changes
✅ new colors look good on light/dark terminal

@cristiand391 cristiand391 merged commit 22071d8 into main Jun 18, 2024
@cristiand391 cristiand391 deleted the sm/refactor-log-colorization branch June 18, 2024 19:15
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