test: Introduce XS CI workflow #2465

kriskowal · 2024-09-20T20:40:00Z

Description

Rigs CI to run tests with xst. We can elaborate on this with a broader XS version vector and add XS tests to packages. This is in anticipation of a new harness262 package that will need xst in CI.

mhofman · 2024-09-21T00:29:00Z

.github/workflows/ci.yml

+      - name: Run yarn build
+        run: yarn build
+
+      - name: Get XS


Here's an idea:

Suggested change

- name: Get XS

- name: Check release

id: check-release

run: |

if curl -f -s -o /dev/null -I -L https://api.github.com/repos/Moddable-OpenSource/moddable/releases/tags/${{ matrix.moddable-version }} then

echo "release=true"

else

echo "release=false"

fi >> $GITHUB_OUTPUT

- name: Get XS

if: steps.check-release.outputs.release == 'true'

With the build step if it's not.

mhofman · 2024-09-21T00:30:29Z

.github/workflows/ci.yml

+  #     - name: Clone XS
+  #       run: git clone https://github.com/moddable-OpenSource/moddable --branch "${{ matrix.moddable-version }}" --depth 1


I'd recommend using actions/checkout, also avoid to checkout as a subfolder of the repo under testing, it tends to complicate things

kriskowal · 2024-09-24T00:46:23Z

@mhofman I took your cue and reworked test-xs so it works with both versions or hashes and uses a precompiled release for the tag if one exists. I ran a CI job to verify both paths worked and settled on just the 5.0.0 release for now. I expect to update the Moddable version matrix to capture different versions in the fullness of time.

michaelfig

Nice to see some Endo tests getting run by XS! This'll be good to land.

I'm approving conditionally on you deciding to confirm or deny my suggested changes. From that point a passing CI is all I'm looking for.

michaelfig · 2024-09-24T03:37:48Z

.github/workflows/ci.yml

+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4


I'm not sure if you were aware of this, but actions/checkout@v4 has the following default for `path':

# Relative path under $GITHUB_WORKSPACE to place the repository path: ''

So this means the checkouts you do later with path: moddable are checking out to $GITHUB_WORKSPACE/moddable.

If you meant for the endo and moddable checkouts to be siblings (as I understand @mhofman was suggesting) instead of moddable being a subdirectory of the endo checkout, you need something like:

Suggested change

uses: actions/checkout@v4

uses: actions/checkout@v4

path: endo

I'll make further suggestions for changes along that line. If the status quo is sufficient, feel free to ignore those suggestions.

Indeed I was suggesting for them to be siblings.

michaelfig · 2024-09-24T03:38:10Z

.github/workflows/ci.yml

+      - name: Install dependencies
+        run: yarn install --immutable


Suggested change

- name: Install dependencies

run: yarn install --immutable

- name: Install dependencies

run: yarn install --immutable

working-directory: endo

michaelfig · 2024-09-24T03:44:34Z

.github/workflows/ci.yml

+      - name: Run yarn build
+        run: yarn build


Suggested change

- name: Run yarn build

run: yarn build

- name: Run yarn build

run: yarn build

working-directory: endo

michaelfig · 2024-09-24T03:45:06Z

.github/workflows/ci.yml

+
+      - name: Download XS
+        if: steps.check-release.outputs.release == 'download'
+        working-directory: ..


Suggested change

working-directory: ..

michaelfig · 2024-09-24T03:46:03Z

.github/workflows/ci.yml

+      - name: Run XS tests
+        run: |
+          PATH=$PATH:$GITHUB_WORKSPACE/moddable/build/bin/lin/debug
+          yarn lerna run test:xs


Suggested change

- name: Run XS tests

run: |

PATH=$PATH:$GITHUB_WORKSPACE/moddable/build/bin/lin/debug

yarn lerna run test:xs

- name: Run XS tests

run: |

PATH=$PATH:$GITHUB_WORKSPACE/moddable/build/bin/lin/debug

yarn lerna run test:xs

working-directory: endo

michaelfig · 2024-09-24T03:47:48Z

.github/workflows/ci.yml

+        if: steps.check-release.outputs.release == 'build'
+        working-directory: moddable/xs/makefiles/lin
+        run: |
+          make debug MODDABLE=$GITHUB_WORKSPACE/moddable CC='cc "-D__has_builtin(x)=1"' # give the syntax highlighter a hand: '


Suggested change

make debug MODDABLE=$GITHUB_WORKSPACE/moddable CC='cc "-D__has_builtin(x)=1"' # give the syntax highlighter a hand: '

make debug MODDABLE=$GITHUB_WORKSPACE/moddable CC='cc "-D__has_builtin(x)=1"' # here's a nickle, kid... go get yourself a better syntax highlighter

And that’s the difference between giving your 2¢ and giving your 5¢.

mhofman

Approved in principle, I'll let you decide whether to entertain my further suggestions.

mhofman · 2024-09-24T04:45:21Z

.github/workflows/ci.yml

+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4


Indeed I was suggesting for them to be siblings.

mhofman · 2024-09-24T04:50:11Z

.github/workflows/ci.yml

+        if: steps.check-release.outputs.release == 'download'
+        working-directory: ..
+        run: |
+          BINDIR=$GITHUB_WORKSPACE/moddable/build/bin/lin/debug


I'm a little uncertain about downloading the binary to the output of the compilation. Have you considered making a generic $GITHUB_WORKSPACE/bin and downloading the binary there. For the compile version we can simply copy the compiled binary there, and even blow away the checked out folder.

I considered both and flipped a coin. But, so long as I’m making endo and moddable peers under the workspace, I may as well make a bin peer.

mhofman · 2024-09-24T04:59:01Z

.github/workflows/ci.yml

+      - name: Choose download or build
+        id: check-release


Since you seem to be entertaining my suggestions, here is another one: use actions/cache to cache the binary (downloaded or compiled) based on the version.

Something like:

Suggested change

- name: Choose download or build

id: check-release

- name: Restore XS binary

id: restore-xs-release

uses: actions/cache@v4

with:

path: bin/xst

key: xst-lin64-${{ matrix.moddable-version }}

- name: Choose download or build

id: check-release

if: steps.restore-xs-release.outputs.cache-hit != 'true'

(assuming we copied the binary to $GITHUB_WORKSPACE/bin/xst)

Very much entertaining suggestions. If you have a suggestion that would parameterize the architecture, that’ll make it easier to turn on darwin and the holes-in-sides-of-houses variants in CI maybe someday.

There are, but not sure which are effectively needed, since GH cache is supposed to be different by default based on the OS (see enableCrossOsArchive). Here is an example in agoric-sdk:

${{ runner.os }}-${{ runner.arch }}

A relevant PR/commit might be Agoric/agoric-sdk@c9222a9

For the record, I’ve got a good idea of how to add platforms to the matrix and am electing not to at this juncture.

Refs: #2463, #2252 ## Description In #2463 we introduced ModuleSource to the SES permits, but this interacted catastrophically with the native XS ModuleSource. We reverted this change #2468 to unbreak Agoric SDK integration. This change reintroduces the ModuleSource permits, such that they are compatible with both XS and the `@endo/module-source/shim.js`, which anticipates the introduction of an AbstractModuleSource base class. Because SES can more gracefully tolerate the absence of an permitted [[Proto]] than the presence of a non-permitted [[Proto]], this adjusts the shim to ensure that the AbstractModuleSource shape exists as a side-effect of repairs/taming, before permits are applied. ### Security Considerations Increase in memory safety exposure in native code implementation of ModuleSource where applicable. ### Scaling Considerations None. ### Documentation Considerations This change reintroduces a note in NEWS.md for the next release. ### Testing Considerations The prior regression went unnoticed because we did not yet have CI for XS #2465. With this change, `yarn test:xs` in SES validates the shim on XS. We also test `@endo/module-source/shim.js` in `ses/test/module-source.test.js` on Node.js. ### Compatibility Considerations This change is designed to tolerate either path forward for the language, whether or not it accepts an AbstractModuleSource base class. ### Upgrade Considerations None.

kriskowal · 2024-10-04T20:56:46Z

@leotm This can be used as a template for making a CI job that will run test:hermes scripts in CI.

kriskowal force-pushed the kriskowal-xs-ci branch 12 times, most recently from a21437e to 713abb1 Compare September 20, 2024 22:31

kriskowal requested a review from michaelfig September 20, 2024 22:35

kriskowal marked this pull request as ready for review September 20, 2024 22:35

kriskowal force-pushed the kriskowal-xs-ci branch 4 times, most recently from 004dd04 to 061240f Compare September 21, 2024 00:08

mhofman reviewed Sep 21, 2024

View reviewed changes

kriskowal mentioned this pull request Sep 21, 2024

Using XS native Compartments for Agoric Vat Workers #400

Open

29 tasks

kriskowal force-pushed the kriskowal-xs-ci branch 4 times, most recently from 2ed5693 to 51b25fd Compare September 24, 2024 00:29

kriskowal requested a review from mhofman September 24, 2024 00:43

kriskowal force-pushed the kriskowal-xs-ci branch from 51b25fd to 4e42535 Compare September 24, 2024 00:44

michaelfig approved these changes Sep 24, 2024

View reviewed changes

mhofman approved these changes Sep 24, 2024

View reviewed changes

kriskowal force-pushed the kriskowal-xs-ci branch 2 times, most recently from f5d92c0 to b9a8180 Compare September 24, 2024 19:13

kriskowal added 2 commits September 24, 2024 13:00

test: Introduce XS CI workflow

551a714

test(ses): Add minimal XS validation for SES shim

21d4275

kriskowal force-pushed the kriskowal-xs-ci branch from 92f62ea to 21d4275 Compare September 24, 2024 20:00

kriskowal enabled auto-merge September 24, 2024 20:02

kriskowal merged commit 392e9b2 into master Sep 24, 2024
16 checks passed

kriskowal deleted the kriskowal-xs-ci branch September 24, 2024 20:10

This was referenced Sep 24, 2024

feat(ses): Tame ModuleSource #2469

Merged

integrate XS into SES-shim ci build / test #508

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Introduce XS CI workflow #2465

test: Introduce XS CI workflow #2465

kriskowal commented Sep 20, 2024

mhofman Sep 21, 2024

mhofman Sep 21, 2024

kriskowal commented Sep 24, 2024

michaelfig left a comment

michaelfig Sep 24, 2024

mhofman Sep 24, 2024

kriskowal Sep 24, 2024

michaelfig Sep 24, 2024

michaelfig Sep 24, 2024

michaelfig Sep 24, 2024

michaelfig Sep 24, 2024

michaelfig Sep 24, 2024

kriskowal Sep 24, 2024

mhofman left a comment

mhofman Sep 24, 2024

mhofman Sep 24, 2024

kriskowal Sep 24, 2024

mhofman Sep 24, 2024

kriskowal Sep 24, 2024

mhofman Sep 24, 2024

kriskowal Sep 24, 2024

kriskowal commented Oct 4, 2024

-      - name: Get XS
+      - name: Check release
+        id: check-release
+        run: |
+          if curl -f -s -o /dev/null -I -L https://api.github.com/repos/Moddable-OpenSource/moddable/releases/tags/${{ matrix.moddable-version }} then
+              echo "release=true"
+          else
+              echo "release=false"
+          fi >> $GITHUB_OUTPUT
+      - name: Get XS
+        if: steps.check-release.outputs.release == 'true'

		# - name: Clone XS
		# run: git clone https://github.com/moddable-OpenSource/moddable --branch "${{ matrix.moddable-version }}" --depth 1

	uses: actions/checkout@v4
	uses: actions/checkout@v4
	path: endo

	make debug MODDABLE=$GITHUB_WORKSPACE/moddable CC='cc "-D__has_builtin(x)=1"' # give the syntax highlighter a hand: '
	make debug MODDABLE=$GITHUB_WORKSPACE/moddable CC='cc "-D__has_builtin(x)=1"' # here's a nickle, kid... go get yourself a better syntax highlighter

-      - name: Choose download or build
-        id: check-release
+      - name: Restore XS binary
+        id: restore-xs-release
+        uses: actions/cache@v4
+        with:
+          path: bin/xst
+          key: xst-lin64-${{ matrix.moddable-version }}
+      - name: Choose download or build
+        id: check-release
+        if: steps.restore-xs-release.outputs.cache-hit != 'true'

test: Introduce XS CI workflow #2465

test: Introduce XS CI workflow #2465

Conversation

kriskowal commented Sep 20, 2024

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kriskowal commented Sep 24, 2024

michaelfig left a 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

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

mhofman left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kriskowal commented Oct 4, 2024