Skip to content

chore(tests): run verdaccio in-process - #248

Merged
jsteinich merged 2 commits into
open-constructs:mainfrom
gabegorelick:local-registry-process
Jun 10, 2026
Merged

chore(tests): run verdaccio in-process#248
jsteinich merged 2 commits into
open-constructs:mainfrom
gabegorelick:local-registry-process

Conversation

@gabegorelick

@gabegorelick gabegorelick commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

We currently run verdaccio by spawning an npm run process. But since local-registry is a JS file now (#202), we can just call verdaccio's JS API directly.

See #202 (comment)

Related issue

Fixes #

Description

In plain English, describe your approach to addressing the issue linked above. For example, if you made a particular design decision, let us know why you chose this path instead of another solution.

Checklist

  • I have updated the PR title to match CDKTN's style guide
  • I have run the linter on my code locally
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works if applicable
  • New and existing unit tests pass locally with my changes

gabegorelick
gabegorelick commented Jun 7, 2026
View reviewed changes
Comment thread test/local-registry.mjs Resolved
gabegorelick
gabegorelick commented Jun 7, 2026
View reviewed changes
Comment thread test/local-registry.mjs Resolved
gabegorelick
gabegorelick commented Jun 7, 2026
View reviewed changes
Comment thread test/local-registry.mjs Resolved
gabegorelick
gabegorelick commented Jun 7, 2026
View reviewed changes
Comment thread .copywrite.hcl Resolved
gabegorelick
gabegorelick commented Jun 7, 2026
View reviewed changes
Comment thread knip.jsonc Resolved
@gabegorelick
gabegorelick force-pushed the local-registry-process branch from 24769a6 to f57d37e Compare June 7, 2026 05:07
@gabegorelick
gabegorelick marked this pull request as ready for review June 7, 2026 05:09
@gabegorelick
gabegorelick requested a review from a team as a code owner June 7, 2026 05:09
@gabegorelick gabegorelick mentioned this pull request Jun 7, 2026
Merged
7 of 8 tasks
@X-Guardian

X-Guardian commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

I don't think we should be moving code back into bash scripts. The whole point of the original change was to move away from from these. The node script can manage the whole service, something like:

  • local-registry.mjs start calls runServer, then internally waits for /-/ping then daemonises itself and records the child PID to the state file.
  • local-registry.mjs stop reads the state file, SIGTERMs the child, restores the npm registry

The run-against-dist script then goes back to:

trap 'node "$scriptdir"/local-registry.mjs stop || true' EXIT
node "$scriptdir"/local-registry.mjs start "$scriptdir"/verdaccio.yaml
...
node "$scriptdir"/local-registry.mjs stop

@gabegorelick

gabegorelick commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

I don't think we should be moving code back into bash scripts. The whole point of the original change was to move away from from these.

If that's the goal, then why not move run-against-dist to JS? This PR gets us closer to that, by removing the funky process state file management.

Should I go ahead and do that? Or is there a reason we need run-against-dist to remain bash?

@jsteinich

jsteinich commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

If that's the goal, then why not move run-against-dist to JS? This PR gets us closer to that, by removing the funky process state file management.

Should I go ahead and do that? Or is there a reason we need run-against-dist to remain bash?

I am definitely supportive of moving away from bash entirely.

@X-Guardian

X-Guardian commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Yep. No reason to have bash scripts in a Node project.

gabegorelick
gabegorelick commented Jun 8, 2026
View reviewed changes
Comment thread test/run-against-dist.mjs Resolved
gabegorelick
gabegorelick commented Jun 8, 2026
View reviewed changes
Comment thread test/run-against-dist.bat Resolved
gabegorelick
gabegorelick commented Jun 8, 2026
View reviewed changes
Comment thread test/run-against-dist.mjs Resolved
gabegorelick
gabegorelick commented Jun 8, 2026
View reviewed changes
Comment thread test/test-helper.ts Resolved
@gabegorelick
gabegorelick force-pushed the local-registry-process branch from bed9e7b to dbbd12a Compare June 8, 2026 18:41
gabegorelick
gabegorelick commented Jun 8, 2026
View reviewed changes
Comment thread package.json Resolved
@gabegorelick

gabegorelick commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Updated to port run-against-dist to JS.

@gabegorelick
gabegorelick force-pushed the local-registry-process branch from dbbd12a to be726ec Compare June 8, 2026 20:56
@gabegorelick

gabegorelick commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, it's taking some back and forth to get the Windows tests working since I don't run Windows locally. Let me put this PR into draft while I fix it.

@gabegorelick
gabegorelick marked this pull request as draft June 8, 2026 22:08
@gabegorelick
gabegorelick force-pushed the local-registry-process branch 3 times, most recently from ef1273e to abc769d Compare June 9, 2026 14:22
@gabegorelick
gabegorelick marked this pull request as ready for review June 9, 2026 15:08
@gabegorelick

gabegorelick commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

I think the only remaining test failures are flakes. So this is ready for review.

gabegorelick
gabegorelick commented Jun 9, 2026
View reviewed changes
Comment thread test/provider-tests/test-provider.bat Resolved
@gabegorelick gabegorelick mentioned this pull request Jun 10, 2026
Closed
1 task done
@gabegorelick
chore(tests): run verdaccio in-process
8777296 
We currently run verdaccio by spawning an `npm run` process. But since
`local-registry` is a JS file now (open-constructs#202), we can just call verdaccio's
JS API directly.

See open-constructs#202 (comment)
@gabegorelick
chore(test): rewrite run-against-dist in JS
a4fa348
@gabegorelick
gabegorelick force-pushed the local-registry-process branch from abc769d to a4fa348 Compare June 10, 2026 02:49
@jsteinich
jsteinich enabled auto-merge (squash) June 10, 2026 02:59
@jsteinich
jsteinich merged commit c17677d into open-constructs:main Jun 10, 2026
256 checks passed
@open-constructs-cdktn open-constructs-cdktn Bot mentioned this pull request Jun 10, 2026
Merged
so0k pushed a commit that referenced this pull request Jul 3, 2026
@open-constructs-cdktn
chore(release): release v0.23.4 (#254)
bf3131c 
🤖 Release PR — merge to cut a new release. Kept open and rebased
as commits land on `main`.
---


<details><summary>0.23.4</summary>

##
[0.23.4](v0.23.3...v0.23.4)
(2026-07-02)


### Features

* **cli:** validate Terraform CLI feature versions
([#237](#237))
([32d434c](32d434c))
* declared Terraform/OpenTofu target versions (validation foundation)
([#269](#269))
([0aa2343](0aa2343))
* **lib:** S3-backend - Add `useLockfile` field and update
`dynamodbTable` deprecated JSDoc
([#234](#234))
([32a408f](32a408f))
* **lib:** validate Fn usage against Terraform/OpenTofu versions
([#268](#268))
([cdf8115](cdf8115))


### Bug Fixes

* **cli:** don't mistake terraform error output for a missing-variable
prompt
([#267](#267))
([0ea946f](0ea946f))
* **cli:** release terraform state lock when interrupting diff/deploy
([#284](#284))
([de617db](de617db))
* **cli:** resolve prebuilt Java provider versions via repo1.maven.org
([#285](#285))
([d5af07d](d5af07d))
* **gha:** Adding CHANGELOG.md to prettier ignore list
([#258](#258))
([5160469](5160469))
* **gha:** pull jsii-terraform CI image from GHCR instead of Docker Hub
([#261](#261))
([f19d34b](f19d34b))
* **provider-generator:** copy only publishable files into the jsii
compile bundle
([#292](#292))
([562d4c7](562d4c7))
* **provider-generator:** match shorthand provider sources against
OpenTofu schema keys
([#293](#293))
([fda0103](fda0103))
* **release:** add Changelog title for correct release-please insertion
([#255](#255))
([004818a](004818a))
* **release:** drop devEngines.packageManager that broke npm publish
([ef508a6](ef508a6))
* **tools:** repair generate-function-bindings tool
([#273](#273))
([821e1cf](821e1cf))


### Miscellaneous Chores

* Add Gradle to Mise config
([#271](#271))
([1578270](1578270))
* add pnpm to mise
([#266](#266))
([48ab91f](48ab91f))
* add uv to mise
([#260](#260))
([163cf3d](163cf3d))
* **deps:** bump nrwl/nx-set-shas from
38457b511e60ee4e1da09255b658e480748a1af4 to
afb73a62d26e41464e9254689e1fd6122ee683c1
([#236](#236))
([b5ac018](b5ac018))
* **deps:** bump the github-actions-backward-compatible group across 1
directory with 7 updates
([#235](#235))
([de67d9f](de67d9f))
* **deps:** replace uuid dependency with node:crypto.randomUUID
([#291](#291))
([6d3657a](6d3657a))
* ESLint ignore test edge-provider-bindings
([#270](#270))
([ba407bb](ba407bb))
* Fix Verdaccio logging config
([#272](#272))
([aded64b](aded64b))
* **gha:** parallelize provider binding generation for documentation
examples
([#244](#244))
([0a9c955](0a9c955))
* Migrate package manager from yarn to pnpm
([#170](#170))
([d23da4e](d23da4e))
* migrate planning artifacts to cdktn-planning [skip ci]
([968c358](968c358))
* **release:** add release-please automation for versioning
([#243](#243))
([c759eec](c759eec))
* Remove dependency-pinner tool
([#250](#250))
([42c27d5](42c27d5))
* Remove unused dependencies
([#281](#281))
([c7de65e](c7de65e))
* **tests:** run verdaccio in-process
([#248](#248))
([c17677d](c17677d))
* Update JSII version constraint
([#245](#245))
([d89558f](d89558f))
* Update scheduled workflows to only run on open-constructs/cdk-terrain
([#256](#256))
([ba408cf](ba408cf))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: open-constructs-cdktn[bot] <291052431+open-constructs-cdktn[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jsteinich jsteinich jsteinich approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gabegorelick @X-Guardian @jsteinich