Skip to content

feat(test-nx-utils): add testing lib for nx specific logic #777

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

Merged
merged 7 commits into from
Aug 5, 2024

Conversation

BioPhoton
Copy link
Collaborator

@BioPhoton BioPhoton commented Aug 2, 2024

This PR adds a new testing package the encapsulate nx dependencies.

It includes:

  • new package test-nx-utils to group Nx related dependencies
    • new helper to generate files

@github-actions github-actions bot added 📖 Project documentation improvements or additions to the project documentation 🔬 testing writing tests 🧩 nx-plugin 🛠️ tooling labels Aug 2, 2024
Copy link

github-actions bot commented Aug 2, 2024

Code PushUp

🤨 Code PushUp report has both improvements and regressions – compared target commit 5259341 with source commit 48f2875.

🕵️ See full comparison in Code PushUp portal 🔍

🏷️ Categories

🏷️ Category ⭐ Previous score ⭐ Current score 🔄 Score change
Performance 🟡 58 🟡 57 ↓ −0.4
Code coverage 🟡 90 🟡 89 ↓ −0.1
Custom checks 🟡 67 🟡 67
Security 🟢 100 🟢 100
Updates 🟡 78 🟡 78
Accessibility 🟢 91 🟢 91
Best Practices 🟢 100 🟢 100
SEO 🟡 61 🟡 61
Bug prevention 🟢 100 🟢 100
Code style 🟢 99 🟢 99

🗃️ Groups

👎 2 groups regressed
🔌 Plugin 🗃️ Group ⭐ Previous score ⭐ Current score 🔄 Score change
Lighthouse Performance 🟡 58 🟡 57 ↓ −0.4
Code coverage Code coverage metrics 🟡 90 🟡 89 ↓ −0.1

18 other groups are unchanged.

🛡️ Audits

👍 5 audits improved, 👎 3 audits regressed, 13 audits changed without impacting score
🔌 Plugin 🛡️ Audit 📏 Previous value 📏 Current value 🔄 Value change
Lighthouse Speed Index 🟨 5.2 s 🟥 6.0 s ↑ +14 %
Lighthouse Time to Interactive 🟥 14.6 s 🟥 17.9 s ↑ +22 %
Lighthouse First Contentful Paint 🟨 2.8 s 🟨 2.7 s ↓ −3 %
Lighthouse Largest Contentful Paint 🟨 2.8 s 🟨 2.7 s ↓ −3 %
Lighthouse First Meaningful Paint 🟨 2.8 s 🟨 2.7 s ↓ −3 %
Code coverage Line coverage 🟩 91.2 % 🟩 91.4 % ↑ +0 %
Code coverage Function coverage 🟩 92.1 % 🟩 91.9 % ↓ +0 %
Code coverage Branch coverage 🟨 83.9 % 🟨 83.9 % ↑ +0 %
Lighthouse Minimizes main-thread work 🟥 16.8 s 🟥 21.2 s ↑ +26 %
Lighthouse Avoids enormous network payloads 🟩 Total size was 1,792 KiB 🟩 Total size was 1,796 KiB ↑ +0 %
Lighthouse JavaScript execution time 🟥 7.2 s 🟥 10.5 s ↑ +46 %
Lighthouse Metrics 🟩 14592 🟩 17858 ↑ +22 %
Lighthouse Total Blocking Time 🟥 4,840 ms 🟥 7,920 ms ↑ +64 %
Lighthouse Max Potential First Input Delay 🟥 2,360 ms 🟥 3,750 ms ↑ +59 %
Lighthouse Eliminate render-blocking resources 🟥 Potential savings of 990 ms 🟥 Potential savings of 730 ms ↓ −26 %
Lighthouse Uses efficient cache policy on static assets 🟨 27 resources found 🟨 27 resources found ↑ +0 %
Lighthouse Server Backend Latencies 🟩 100 ms 🟩 220 ms ↑ +117 %
Lighthouse Initial server response time was short 🟩 Root document took 330 ms 🟩 Root document took 380 ms ↑ +17 %
Lighthouse Reduce unused CSS 🟥 Potential savings of 105 KiB 🟥 Potential savings of 105 KiB ↑ +5 %
Lighthouse Network Round Trip Times 🟩 20 ms 🟩 20 ms ↓ −20 %
Lighthouse Cumulative Layout Shift 🟩 0 🟩 0 ↑ +∞ %

487 other audits are unchanged.

@BioPhoton BioPhoton marked this pull request as ready for review August 2, 2024 13:02
@BioPhoton BioPhoton requested a review from matejchalk August 2, 2024 13:02
@BioPhoton BioPhoton merged commit abe4f4f into main Aug 5, 2024
30 checks passed
@BioPhoton BioPhoton deleted the create-nx-testing-helper branch August 5, 2024 11:03
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.

Nice prep for testing nx. 👏 Did not get to this earlier so at least added a few comments now.

it('should create files from tree', async () => {
const root = join(baseDir, 'materialize');

const tree = createTreeWithEmptyWorkspace({ layout: 'apps-libs' });
Copy link
Collaborator

Choose a reason for hiding this comment

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

🧪 Output cleanup: While the tmp folder is ignored by default, each test should clean up after itself (the strategy is on our Wiki). I recommend adding an after hook that removes the materialize subfolder to ensure clean state.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, I missed that 🙈 Yes, the test should clean up the file system changes after it runs 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

🧪 Utils structure: Wondering if it would not be better to keep this in a subfolder of test-utils (the idea was only to separate utilities and mock setup). Then there would not need to be separate config files and targets.


registerPluginInWorkspace(tree, 'nx-plugin');

const nxJson = JSON.parse(tree.read('nx.json')?.toString() ?? '{}');
Copy link
Collaborator

Choose a reason for hiding this comment

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

🧪 Linear flow: There should be no need for conditional statements in tests. The test should know what is expected and not insert any default values such as {}. I would suggest using ! here. Curious to hear what @matejchalk thinks.

This is applicable to multiple places.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Completely agree, this should be JSON.parse(tree.read('nx.json')!.toString()).


describe('executorContext', () => {
it('should create context for given project name', () => {
expect(executorContext('my-lib')).toStrictEqual({
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚙️ TypeScript: To get auto-complete if there is a non-trivial type to match, you can use satisfies or a type argument. Applicable in multiple places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧩 nx-plugin 📖 Project documentation improvements or additions to the project documentation 🔬 testing writing tests 🛠️ tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants