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: remove ansi characters in steps statusDetails #1085

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

todti
Copy link
Contributor

@todti todti commented Jul 31, 2024

Context

Checklist

@todti todti requested a review from epszaw July 31, 2024 21:30
@github-actions github-actions bot added the theme:api Javascript API related issue label Jul 31, 2024
@todti todti added the type:bug Something isn't working label Jul 31, 2024
@todti todti force-pushed the fix_ansi_in_statusDetails branch from 051bfe8 to 6232946 Compare July 31, 2024 21:40
Copy link
Member

@epszaw epszaw left a comment

Choose a reason for hiding this comment

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

The changes look good. The only one comment I left there is optional to fix

@@ -184,16 +184,15 @@ export abstract class MessageTestRuntime implements TestRuntime {

return result;
} catch (err) {
const { message, stack } = err as Error;
const details = getMessageAndTraceFromError(err as Error);

await this.sendMessage({
type: "step_stop",
data: {
status: getStatusFromError(err as Error),
stop: Date.now(),
statusDetails: {
Copy link
Member

Choose a reason for hiding this comment

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

You can just assign details or call the function right in the object:

// ...
statusDetails: getMessageAndTraceFromError(err as Error);
// ...

@baev baev merged commit 4b5fbd8 into main Aug 1, 2024
10 checks passed
@baev baev deleted the fix_ansi_in_statusDetails branch August 1, 2024 10:11
@delatrie
Copy link
Collaborator

delatrie commented Aug 1, 2024

That fixes the problem in the following packages, which use MessageTestRuntime under the hood:

  • allure-cucumberjs
  • allure-codeceptjs
  • allure-jasmine
  • allure-jest
  • allure-mocha
  • allure-vitest

Allure-cypress might still be affected.
Allure-playwright already uses the correct implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme:api Javascript API related issue type:bug Something isn't working
Projects
None yet
4 participants