MWPW-163603 benchmark card rendering time in PR #3377

npeltier · 2024-12-13T17:08:46Z

benchmark page (only CCD for now)
250ms limit in:
/libs/features/mas/docs/benchmarks.html
adjusted to network timing in :
/libs/features/mas/docs/benchmarks.html?adjust=true

There is still a risk that some perf degradation passes tests if they are run on slow networks, but that's all we can do atm.

Test URLs:

aem-code-sync · 2024-12-13T17:09:13Z

	Page	Scores	Audits	Google
📱	/?martech=off
🖥️	/?martech=off

codecov · 2024-12-13T17:09:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.46%. Comparing base (5bbb0bf) to head (7a8385a).
Report is 6 commits behind head on stage.

Additional details and impacted files

@@           Coverage Diff           @@
##            stage    #3377   +/-   ##
=======================================
  Coverage   96.46%   96.46%           
=======================================
  Files         255      255           
  Lines       59419    59430   +11     
=======================================
+ Hits        57318    57329   +11     
  Misses       2101     2101

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

3ch023 · 2024-12-19T12:12:49Z

nala/features/mas/benchmark/benchmark.test.js

+
+test.describe('Benchmark feature test suite', () => {
+  test(`${features[0].name},${features[0].tags}`, async ({ page, baseURL }) => {
+    const testPage = `${baseURL}${features[0].path}${miloLibs}`;


I do have a concern including this in Milo nala tests.
If freya or wcs cdn will be causing slower render it will fail Milo nala tests and block developers until someone will comment this test out.
We could try it out and if we trues in Freya/WCS stability it would work, however I think we should notify people so that they are aware about this test having external dependencies (it's ok for Nala tests)
And if this test will be flaky we will have to take it out and maybe run it only manually/before release.
We should make sure we don't create stress for Milo community with this

I agree, this is why i added alignement with network capabilities. For the rest items should be cached, so this is CDN response time for constant time we are measuring and this should not change much.
I thought about not adding this systematically and/or raise acceptance bar, but i think this will defeat the purpose of us guaranteeing cards performances

3ch023 · 2024-12-19T12:15:58Z

libs/features/mas/docs/common.js

@@ -0,0 +1,40 @@
+import '../../../deps/custom-elements.js';


maybe its better to place it in src folder
and keep docs folder for html build artifacts

actually no, src is for doc build, and docs is for built / as is artifacts, so it's at its place

3ch023 · 2024-12-19T12:17:21Z

libs/features/mas/docs/common.js

+import '../../spectrum-web-components/dist/action-button.js';
+import '../../../features/spectrum-web-components/dist/button.js';
+const init = () => {
+  const ENVS = {


kudos on adding this, I would mention it in PR description

i could yes, i didn't add it to ccd page and i should have, but i'm waiting for @yesil merge on spectrum css before to limit merge work

Oh, I also have something similar but not common but will replace with this once before merge.

3ch023 · 2024-12-19T12:21:06Z

libs/features/mas/docs/common.js

+  });
+  masCommerceService.setAttribute('host-env', 'prod');
+  masCommerceService.setAttribute('lana-tags', 'ccd');
+  masCommerceService.setAttribute('lana-sample-rate', '100'); // optional, default value is set to '1'


including this on every page would pollute our prod logs
ccd page is not throwing errors, but https://mwpw-163603--milo--npeltier.aem.page/libs/features/mas/docs/merch-card.html does

not sure its a big deal, but something worth mentioning

(even if it's 9 views a week)

3ch023

all looks good, except one small thing:
https://mwpw-163603--milo--npeltier.aem.page/libs/features/mas/docs/benchmarks.html is not referenced from anywhere in documentation which makes it not possible to find

what i keep dreaming about is a sidenav with all our pages, but since its an extra effor it might be too much for this PR. just an idea.
at least please add a link somewhere in mas page.

github-actions · 2024-12-20T01:37:40Z

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

yesil · 2024-12-20T09:58:29Z

libs/features/mas/docs/benchmarks.html

+  <meta charset="UTF-8">
+  <title>M@S Benchmarks</title>
+  <script>
+    const start = performance.now();


You can also use mark and measure apis that do not require you to assign values to variables.

performance.mark('commerceStart'); performance.mark('commerceEnd'); performance.measure('commerceLoadTime', 'commerceStart', 'commerceEnd');

yesil · 2024-12-20T10:00:12Z

libs/features/mas/docs/benchmarks.html

+</head>
+
+<script>
+  const measureCardPerformances = async (container, fragmentId) => {


we should probably incorporate performance api to merch-card and aem-fragment custom elements directly.

and use performance api entries while reporting the measurements.

npeltier · 2024-12-20T15:01:42Z

i'll incorporate @yesil feedbacks + will try to post results to some place

github-actions · 2024-12-28T01:25:04Z

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

github-actions · 2025-01-04T01:25:19Z

Closing this PR due to inactivity.

3ch023

needs a small fix for ccd.html

3ch023 · 2025-01-08T13:15:06Z

libs/features/mas/docs/ccd.html

@@ -51,7 +51,20 @@
    </script>
 </head>

+<<<<<<< HEAD


merge issue?
https://mwpw-163603--milo--npeltier.aem.page/libs/features/mas/docs/ccd.html is broken a bit

3ch023

could you try running npm install in /libs/features/mas?
i wonder if it will remove changes in merch-stock.js etc

yesil

LGTM 👍

yesil · 2025-01-09T12:57:30Z

libs/features/mas/docs/ccd.html

-<body class="spectrum spectrum--medium spectrum--light">
-  <main>
+  <body class="spectrum spectrum--medium spectrum--light">
+    <div class="sidenav">


nit: you could use aside for element

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/aside

npeltier and others added 5 commits December 11, 2024 14:59

MWPW-163602 draft

7dfacb2

some fixes

4665edd

add benchmark test to Nala

faa0f9c

fix adjust

92e4449

bring adjusted limit to nala too

247a95f

npeltier added the commerce label Dec 13, 2024

3ch023 reviewed Dec 19, 2024

View reviewed changes

3ch023 requested changes Dec 19, 2024

View reviewed changes

npeltier added 2 commits December 19, 2024 19:27

MWPW-163603 review comments

5d5dde0

forgot to rebuild docs

86a938c

yesil reviewed Dec 20, 2024

View reviewed changes

github-actions bot added the Stale label Dec 28, 2024

github-actions bot closed this Jan 4, 2025

npeltier reopened this Jan 4, 2025

github-actions bot removed the Stale label Jan 5, 2025

Merge branch 'stage' of github.com:adobecom/milo into MWPW-163603

cc45338

3ch023 approved these changes Jan 7, 2025

View reviewed changes

3ch023 requested changes Jan 8, 2025

View reviewed changes

npeltier added 3 commits January 8, 2025 21:04

review comments

10e8ed8

latest

f6f195d

Merge branch 'stage' of github.com:adobecom/milo into MWPW-163603

11d81b9

make sidenav smaller :)

c7c20b1

3ch023 requested changes Jan 9, 2025

View reviewed changes

yet a bit smaller :)

cebf18f

yesil approved these changes Jan 9, 2025

View reviewed changes

npeltier added 2 commits January 9, 2025 14:15

fix benchmark selector

b0ca704

use aside

1d08c47

3ch023 approved these changes Jan 9, 2025

View reviewed changes

desperate try to fool cov

7a8385a

npeltier mentioned this pull request Jan 10, 2025

MWPW-142145: [Milo Plans Page] Plans page Tab behavior #3420

Closed

afmicka added verified PR has been E2E tested by a reviewer Ready for Stage labels Jan 10, 2025

milo-pr-merge bot merged commit b6e2bcd into adobecom:stage Jan 13, 2025
24 checks passed

milo-pr-merge bot mentioned this pull request Jan 13, 2025

[Release] Stage to Main #3468

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MWPW-163603 benchmark card rendering time in PR #3377

MWPW-163603 benchmark card rendering time in PR #3377

npeltier commented Dec 13, 2024 •

edited

Loading

aem-code-sync bot commented Dec 13, 2024 •

edited

Loading

codecov bot commented Dec 13, 2024 •

edited

Loading

3ch023 Dec 19, 2024

npeltier Dec 19, 2024

3ch023 Dec 19, 2024

npeltier Dec 19, 2024

3ch023 Dec 19, 2024

npeltier Dec 19, 2024

yesil Dec 20, 2024 •

edited

Loading

3ch023 Dec 19, 2024 •

edited

Loading

npeltier Dec 19, 2024

3ch023 left a comment

github-actions bot commented Dec 20, 2024

yesil Dec 20, 2024

yesil Dec 20, 2024

npeltier commented Dec 20, 2024

github-actions bot commented Dec 28, 2024

github-actions bot commented Jan 4, 2025

3ch023 left a comment

3ch023 Jan 8, 2025

3ch023 left a comment

yesil left a comment

yesil Jan 9, 2025

npeltier Jan 9, 2025

MWPW-163603 benchmark card rendering time in PR #3377

MWPW-163603 benchmark card rendering time in PR #3377

Conversation

npeltier commented Dec 13, 2024 • edited Loading

aem-code-sync bot commented Dec 13, 2024 • edited Loading

codecov bot commented Dec 13, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yesil Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

3ch023 Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3ch023 left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npeltier commented Dec 20, 2024

github-actions bot commented Dec 28, 2024

github-actions bot commented Jan 4, 2025

3ch023 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3ch023 left a comment

Choose a reason for hiding this comment

yesil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npeltier commented Dec 13, 2024 •

edited

Loading

aem-code-sync bot commented Dec 13, 2024 •

edited

Loading

codecov bot commented Dec 13, 2024 •

edited

Loading

yesil Dec 20, 2024 •

edited

Loading

3ch023 Dec 19, 2024 •

edited

Loading