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

refactor(utils): use cliui as central logger #487

Merged
merged 92 commits into from
Mar 12, 2024
Merged

Conversation

BioPhoton
Copy link
Collaborator

@BioPhoton BioPhoton commented Feb 7, 2024

This PR includes:

Note

I will move #513 into a new PR and implement in when the PR in cliui is in if I reopen ever.
created #552

Testing:
I tried to use hooks for the logger tests to switch to raw mode and cleanup the logs beforeEach run.

I was only able to use this setup in utils which imports directly the file:

Working code:

import {beforeEach, vi} from "vitest";
import {ui} from "../../src/lib/logging";

vi.mock('../../src/lib/logging', async () => {
  const module = await vi.importActual('../../src/lib/logging');

  module.ui().switchMode('raw')
  return module;
});

beforeEach(() => {
   ui().logger.flushLogs();
})

However in core and cli i was only able to have the mode switching in the setup script and hat to move the logger cleanup in the test files. If I import ui from @code-pushup/utils many other tests fail.

Working reduced script:

import {ui} from "@code-pushup/utils";

vi.mock('@code-pushup/utils', async () => {
  const module = await vi.importActual('@code-pushup/utils');

  module.ui().switchMode('raw')
  return module;
});

Update:

I was able to implement the beforeEach hook with async imports:

beforeEach(async () => {
  const {ui} = await vi.importActual<{ ui: () => CliUi }>(
    '@code-pushup/utils',
  );
  ui().logger.flushLogs();
});

closes #502
related #513

@github-actions github-actions bot added 📖 Project documentation improvements or additions to the project documentation 🧩 coverage-plugin and removed 🧩 coverage-plugin labels Feb 15, 2024
@github-actions github-actions bot removed the 📖 Project documentation improvements or additions to the project documentation label Feb 17, 2024
@BioPhoton BioPhoton marked this pull request as ready for review February 18, 2024 14:55
Copy link

nx-cloud bot commented Feb 23, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit fba81fc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 9 targets

Sent with 💌 from NxCloud.

BioPhoton and others added 7 commits March 12, 2024 13:47
…st.ts

Co-authored-by: Katka Pilátová <katerina.pilatova@flowup.cz>
Co-authored-by: Katka Pilátová <katerina.pilatova@flowup.cz>
@BioPhoton BioPhoton requested a review from Tlacenka March 12, 2024 13:13
Tlacenka
Tlacenka previously approved these changes Mar 12, 2024
Copy link
Collaborator

@Tlacenka Tlacenka left a comment

Choose a reason for hiding this comment

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

Almost there!

Once the proper matchers are applied in all applicable places, this PR is good to go from my side!

matejchalk
matejchalk previously approved these changes Mar 12, 2024
# Conflicts:
#	packages/cli/src/lib/print-config/print-config-command.ts
…ble-from-cliui

# Conflicts:
#	packages/utils/src/lib/git.integration.test.ts
Copy link
Collaborator

@Tlacenka Tlacenka left a comment

Choose a reason for hiding this comment

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

🌈

@BioPhoton BioPhoton merged commit 0f9db8a into main Mar 12, 2024
19 checks passed
@BioPhoton BioPhoton deleted the use-table-from-cliui branch March 12, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Muting CLI logger in test output
3 participants