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

Custom matcher for ui logger #552

Closed
6 tasks
BioPhoton opened this issue Mar 8, 2024 · 2 comments · Fixed by #923
Closed
6 tasks

Custom matcher for ui logger #552

BioPhoton opened this issue Mar 8, 2024 · 2 comments · Fixed by #923
Assignees
Labels
💡 good first issue good for newcomers 🆘 help wanted assistance is required 🔬 testing writing tests 🤓 UX UX improvement for CLI users
Milestone

Comments

@BioPhoton
Copy link
Collaborator

BioPhoton commented Mar 8, 2024

User story

As a developer I want to have a good signal to noise ration and want to write my tests with as less boilerplate as possible.
Therefor I need a custom matcher for the UI logger.

Bad example:

// ui().logger.error('Something went wrong');

expect(ui().logger.getRenderer().getLogs()).toStrictEqual([
  {
    stream: 'stderr',
    message: '[ red(error) ] Something went wrong'
  }
])

Suggested matcher:

  • expect(ui()).toHaveLogged('...')
  • expect(ui()).toHaveLoggedNth(1, '...')
  • expect(ui()).not.toHaveLogged())

Conversation: #487 (comment)

Good example:

// ui().logger.error('Something went wrong');

expect(ui()).toHaveLoggedLevelNth(1, 'error');
expect(ui()).toHaveLoggedMessageNth(1, 'Something went wrong');

Acceptance criteria

I want to be able to use the custom matcher in the following libs:

  • vitest
  • jest (optional)

Internal logic:

  • it handles removes ASCII color
    expect(msg.replace(/\u001B\[\d+m/g, '')).toBe("[ blue(info) ] This is is blue in terminal")
  • custom matcher toMatchPlainCharacter instead of removeColorCodes (optional)
    I want to be able to have different matcher present:
  • toHaveLoggedLevel('...')
  • toHaveLoggedLevelNth(1, '...')
  • toHaveLoggedMessage('...')
  • toHaveLoggedMessageNth(1, '...')

Implementation details

import { expect } from 'vitest';
import {cliui} from "@poppinss/cliui";

declare module 'vitest' {
  export interface Assert {
    toHaveLoggedMessage(expected: string): void;
    toHaveLoggedMessageNth(nth: number, expected: string): void;
  }
}

export type CliUi = ReturnType<typeof cliui>;
// Implementation of toHaveLogged
expect.extend({
  toHaveLoggedMessage(received: CliUi, expected: string) {
    const logs = received.logger.getRenderer().getLogs();
    const pass = logs.some(log => log.message === expected);
    return {
      pass,
      message: () => pass
        ? `expected not to have logged: ${expected}`
        : `expected to have logged: ${expected}`,
    };
  },

  toHaveLoggedMessageNth(received: CliUi, nth: number, expected: string) {
    const logs = received.logger.getRenderer().getLogs();
    const pass = (logs[nth - 1]?.message === expected) ?? false;
    return {
      pass,
      message: () => pass
        ? `expected not to have logged at position ${nth}: ${expected}`
        : `expected to have logged at position ${nth}: ${expected}`,
    };
  },
});

Usage in global-setup.ts

import './testing/test-utils/src/lib/matcher/ui-logger';
@BioPhoton BioPhoton changed the title custom matcher for ui logger Custom matcher for ui logger Mar 10, 2024
@BioPhoton BioPhoton added 🔬 testing writing tests 🤓 UX UX improvement for CLI users labels Mar 10, 2024
@BioPhoton BioPhoton added the 💡 good first issue good for newcomers label Mar 10, 2024
@Tlacenka
Copy link
Collaborator

Suggestions:

  • Add a helper function to test-utils that returns Nth log directly as a string.
  • If applicable, distinguish formatted (bold, colour code) and unformatted logs.

@BioPhoton
Copy link
Collaborator Author

BioPhoton commented Mar 20, 2024

Feedback from @matejchalk:

On balance, I'd go with no style testing. The text content is the key part of the assertion for me.
Although if create a custom matcher, we could theoretically support both. E.g. .toHaveLogged('...') would match unstyled text, but .toHaveLoggedStyled('...') would be available for matching styles also. Alternatively, .toHaveLogged('...') could try to match both unstyled and styled text under the hood and only fail if neither matches (bit less transparent, but perhaps more convenient?). 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 good first issue good for newcomers 🆘 help wanted assistance is required 🔬 testing writing tests 🤓 UX UX improvement for CLI users
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants