Added reactor-extension-alloy as a package within alloy

[Spencer-Smith]

Description

• Cloned reactor-extension-alloy into packages/reactor-extension, preserving version history for files
• Updated package.json in that package to get @adobe/alloy dependency from within the workspace
• Replicated reactor-extension-alloy workflows in the workspace root
• Updated pnpm scripts at workspace root to build and run tests in the extension package, added functionality to run unit/integration tests for just a single package

TODO:

• Move reactor-extension-alloy specific secrets to alloy github project
• (?) Unify eslint configs and run against entire project (right now root level lint config does not check reactor-extension package files, and then reactor-extension package runs its own lint config against its files)
• (?) Simplify testing paradigm
• (?) Consolidate workflows
• Test new release process

Related Issue

Jira

Motivation and Context

Screenshots (if appropriate):

N/A

Checklist:

• I have signed the Adobe Open Source CLA or I'm an Adobe employee.
• I have made any necessary test changes and all tests pass.
• I have run the Sandbox successfully.
• I have added a Changeset (pnpm changeset) or it is not necessary because this PR is not consumer-facing.

[carterworks]

the PR was too large and the review page of GitHub experienced performance issues, so all my comments are clumped together. Sorry.

Delete unnecessary files in packages/reactor-extension

• eslint.config.standalone.mjs.bak
• COPYRIGHT
• LICENSE
• CODE_OF_CONDUCT.md
• .nvmrc
• .prettierrc.json
• .github
• merge pnpm-workspace.yaml with top-level one? or is nesting okay?

Fix eslint errors in tests

either adjust the eslint.config.js or deal with them. If there are lots of changes, move this to a separate PR

• alloy/license-header should apply to the reactor-extension package.
• instead of adding package/reactor-extension to the ignores for rulesets, either narrow the files so that it applies to just the SDK or make the reactor-extension compliant. they aren't
• 🚨 If you change nothing else, at least fix this 🚨 alloy/tests was changed from applying to packages/**/test/** to packages/reactor-extension/test/** (aka the linting rules only apply to reactor-extension instead of everything)

`pnpm lint` output

❯ pnpm lint

adobe-alloy-monorepo@0.0.1 lint /Users/cmcbride/homebase/code/github.com/adobe/alloy

eslint --cache --fix ".{js,cjs,mjs,jsx}" "{sandboxes//src,packages//{src,test,scripts},scripts}/**/.{js,cjs,mjs,jsx}"

/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/functional/specs/runtime/redirectWithIdentity.spec.mjs

110:6  warning  Disabled test - if you want to skip a test temporarily, use .todo() instead  vitest/no-disabled-tests
/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/functional/specs/runtime/visitorMigration.spec.mjs

88:6  warning  Disabled test - if you want to skip a test temporarily, use .todo() instead  vitest/no-disabled-tests
/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/functional/specs/view/dataElements/xdmObjectSchemaSelection.spec.mjs

14:33  warning  '/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/functional/helpers/endpointMocks/sandboxesMocks.mjs' imported multiple times  import/no-duplicates

15:31  warning  '/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/functional/helpers/endpointMocks/sandboxesMocks.mjs' imported multiple times  import/no-duplicates
/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/functional/specs/view/runCommonExtensionViewTests.mjs

18:8  warning  Disabled test - if you want to skip a test temporarily, use .todo() instead  vitest/no-disabled-tests
/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/unit/view/components/overrides/utils.spec.js

70:10  warning  Test title must be a string, a function or class name  vitest/valid-title

115:10  warning  Test title must be a string, a function or class name  vitest/valid-title
/Users/cmcbride/homebase/code/github.com/adobe/alloy/packages/reactor-extension/test/unit/view/utils/trimValues.spec.js

87:8  warning  Test title must be a string, a function or class name  vitest/valid-title

Failing tests when run locally

`pnpm test` output

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
FAIL   reactor-extension/unit  packages/reactor-extension/test/unit/lib/generatedAlloy.spec.js > Generated alloy.js (preinstalled mode)

AssertionError: expected false to be true // Object.is equality

Expected


Received


true


false

❯ packages/reactor-extension/test/unit/lib/generatedAlloy.spec.js:42:38

40|     alloyPath = path.join(distPath, "alloyPreinstalled.js");

41|     // eslint-disable-next-line vitest/no-standalone-expect

42|     expect(fs.existsSync(alloyPath)).toBe(true);

|                                      ^

43|

44|     // Load the generated code once
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/5]⎯
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 4 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
FAIL   reactor-extension/integration (chromium)  packages/reactor-extension/test/integration/configuration/advertisingSection.spec.jsx > Config advertising section > updates form values and saves to settings

VitestBrowserElementError: Cannot find element with locator: getByRole('listbox')

  

    

      

        2 options available.
      
      

        1 option available.
      
    
    

  
  

    

      

        

          

            

              

              

            
            

              

              

            
          
        
      
    
  
...
 ❯ Object.selectOption packages/reactor-extension/test/integration/helpers/form.js:125:55
    123|
    124|       // Wait for the listbox with options to appear using Vitest's built-in waiting
    125|       await expect.element(page.getByRole("listbox")).toBeVisible();
       |                                                       ^
    126|
    127|       // Now find and click the matching option
 ❯ packages/reactor-extension/test/integration/configuration/advertisingSection.spec.jsx:151:4
Caused by: Error: Matcher did not succeed in time.

❯ Object.selectOption packages/reactor-extension/test/integration/helpers/form.js:125:7

❯ packages/reactor-extension/test/integration/configuration/advertisingSection.spec.jsx:151:4
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/5]⎯
FAIL   reactor-extension/integration (chromium)  packages/reactor-extension/test/integration/configuration/configOverrideSection.spec.jsx > Config overrides section > allows you to save data elements

TimeoutError: locator.click: Timeout 5000ms exceeded.

Call log:

waiting for locator('[data-vitest="true"]').contentFrame().getByRole('option', { name: 'Enabled' })

locator resolved to 
…


attempting click action

2 × waiting for element to be visible, enabled and stable

element is not stable
retrying click action
waiting 20ms
waiting for element to be visible, enabled and stable
element is not stable


retrying click action

waiting 100ms
waiting for element to be visible, enabled and stable
element is visible, enabled and stable
scrolling into view if needed
done scrolling
element is not visible


retrying click action

waiting 100ms
waiting for element to be visible, enabled and stable


element was detached from the DOM, retrying

❯ Object.selectOption packages/reactor-extension/test/integration/helpers/form.js:136:24

134|         if (text.toLowerCase() === optionText.toLowerCase()) {

135|           // eslint-disable-next-line no-await-in-loop

136|           await option.click();

|                        ^

137|           return;

138|         }

❯ packages/reactor-extension/test/integration/configuration/configOverrideSection.spec.jsx:314:4
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/5]⎯
FAIL   reactor-extension/integration (chromium)  packages/reactor-extension/test/integration/configuration/generalSettingsAndDatastreamSection.spec.jsx > Config general settings and datastream section > sets list form values from settings

VitestBrowserElementError: Cannot find element with locator: getByTestId('productionDatastreamField')

  

    

      

        

          

            

              

              

            
            

              

              

            
          
        
      
    
  
...
 ❯ Object.waitForLoad packages/reactor-extension/test/integration/helpers/form.js:830:45
    828|      */
    829|     waitForLoad: async () => {
    830|       await expect.poll(() => !isLoading()).toBeTruthy();
       |                                             ^
    831|     },
    832|
 ❯ packages/reactor-extension/test/integration/configuration/generalSettingsAndDatastreamSection.spec.jsx:114:26
Caused by: Error: Matcher did not succeed in time.

❯ Object.waitForLoad packages/reactor-extension/test/integration/helpers/form.js:830:7

❯ packages/reactor-extension/test/integration/configuration/generalSettingsAndDatastreamSection.spec.jsx:114:4
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/5]⎯
FAIL   integration (chromium)  packages/browser/test/integration/specs/Command Logic/queueTimeMillis.spec.js > queueTimeMillis > queueTimeMillis reflects time waiting for consent

TypeError: [alloy] [Consent] An error occurred while executing the setConsent command.

Caused by: Network request failed.

Caused by: Failed to fetch

❯ packages/browser/test/integration/specs/Command%20Logic/queueTimeMillis.spec.js:117:5
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[5/5]⎯
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Vitest caught 1 unhandled error during the test run.

This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

TypeError: Network request failed.

Caused by: Failed to fetch

This error originated in "packages/browser/test/integration/specs/Advertising/viewthrough_ids.spec.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.

The latest test that might've caused the error is "should send conversion query with advertising IDs in view-through". It might mean one of the following:

The error was thrown, while Vitest was running this test.
If the error occurred after the test had been completed, this was the last documented test before it was thrown.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Test Files  5 failed | 434 passed (439)

Tests  4 failed | 2619 passed | 11 skipped (2634)

Errors  1 error

Start at  09:32:20

Duration  46.39s (transform 4.35s, setup 24.11s, import 100.88s, tests 295.82s, environment 22.67s)

Changeset publish integration

(maybe you want to do this in a separate PR)
deployExtensionRelease.yml and initializeExtensionRelease.yml both depend on github.com/adobe/project-card-release-automation, which is deprecated since Github got rid of the kanban style "projects" that it used.
Instead, the ssh-ing and building and maybe even uploading should be converted into a script that is run when pnpm publish is run (which is what changesets does when it decides that the version should be bumped). Look at scripts/deploy.sh
You can also delete scrips/version.sh and scripts/updateAlloy.js because that is handled by changesets

Unify PR quality checking tasks

(Personal opinion and nitpick, so accept or ignore with no consequences. Could even be done as a separate PR)
The extension's quality-checks.yml runs build, lint, and format:check in parallel before running unit and integration tests in parallel.
The monorepo's dev.yaml first runs lint then runs all the tests, using vitest's parallelization.
I like the structure of the extension's quality-checks.yml better, so I think that the following should happen:

• Delete dev.yaml-
• Move packages/reactor-extension/.github/workflows/quality-checks.yml into .github/workflows
• Modify the tests steps so that it just rungs pnpm test instead of the test matrix

npm script cleanup

• test:extension should be vitest run --project=reactor-extension/unit --project=reactor-extension/integration since vitest is configured at the root
  • delete test and test:{coverage,unit,integration}* from packages/reactor-extension/package.json
  • delete all test infrastructure, files, scripts, and dependencies, except the functional tests (sauce labs, testcafe)
• delete format, format:check, add-license (and package/reactor-extension/scripts/add-license.mjs) lint, precommit-msg, prepare, prepush-msh`

[carterworks]

Also it doesn't seem like Sonar is picking up on these changes quite yet.

https://aepoperations.cq.corp.adobe.com/code?id=alloy&pullRequest=1448

[Spencer-Smith]

@carterworks I've addressed your requested changes other than unifying the changeset publication (I'd prefer to leave it sort it out separately)

[carterworks]

Files not deleted:

• packages/reactor-extension/.nvmrc still exists
• packages/reactor-extension/pnpm-lock.yaml still exists (979k, stale)

ESLint issues (🚨 critical flagged item not fixed):

• eslint.config.js:303 — alloy/tests still has files: ["packages/reactor-extension/test//*.{cjs,js,jsx}"] (should be packages//test/**)
• eslint.config.js:60 — alloy/license-header still ignores packages/reactor-extension/**
• eslint.config.js:154 — alloy/strict still ignores packages/reactor-extension/**

Broken reference: package.json:33 has "version": "./scripts/version.sh" but that script was deleted

[carterworks]

You've addressed all the things I brought up previously, but pnpm lint fails as do a few of the tests

linting errors

func-style errors — these are now caught because alloy/strict no longer excludes reactor-extension. Need to convert function declarations to function expressions (or arrow functions) at:

• packages/reactor-extension/src/view/actions/updateVariableView.jsx:386

• packages/reactor-extension/src/view/components/extensionView.jsx:185

• packages/reactor-extension/src/view/dataElements/xdmObjectView.jsx:297

• packages/reactor-extension/src/view/utils/trimValues.js:32

• packages/reactor-extension/src/view/utils/useReportAsyncError.js:30

I think there are flaky tests in packages/reactor-extension/test/integration/configuration/configOverrideSection.spec.jsx. find them and add a .skip for now.

[Spencer-Smith]

Linting step should maybe be added to Husky, if it's required for merge? It seems like at least once per PR I add a commit saying "fix linting errors" 😓

[carterworks]

Linting step should maybe be added to Husky, if it's required for merge? It seems like at least once per PR I add a commit saying "fix linting errors" 😓

sure, let's add it. here or in a separate pr–your call.

[carterworks]

Thoughts on the flakey tests?

[Spencer-Smith]

From offline discussion with Carter: will update Husky and flaky test in separate PRs. Also (from before) will handle integrating release notes automation. For now just want to merge this before another state change in extension repo or here causes another break.

I also have done some quick offline testing to confirm that the sandboxes for the root and the extension package are still functioning as intended. I wasn't comprehensive but just wanted to confirm that things were essentially still working, between manual testing and our testing suites I have more confidence this won't introduce any major regressions.