-
Notifications
You must be signed in to change notification settings - Fork 89
feat(integration-tests): Add end-to-end test workflow for downstream clp-ffi-js; Bump yscope-dev-utils to y-scope/yscope-dev-utils@e2a1aed.
#2015
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
7d70548
b06105c
3b28ce6
94e3a2b
8a8feb2
f336c64
eedf82e
999f11a
90e9647
a47a3f0
8eb916f
8956e06
9bfb82e
ab0507d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,95 @@ | ||||||||||||||||||||||
| version: "3" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| includes: | ||||||||||||||||||||||
| yscope-dev-utils: "../../tools/yscope-dev-utils/exports/taskfiles/utils/utils.yaml" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| vars: | ||||||||||||||||||||||
| G_CLP_FFI_JS_SRC_DIR: "{{.G_BUILD_DIR}}/clp-ffi-js" | ||||||||||||||||||||||
| G_CLP_FFI_JS_BUILD_DIR: "{{.G_CLP_FFI_JS_SRC_DIR}}/build/clp-ffi-js" | ||||||||||||||||||||||
| G_CLP_FFI_JS_NODE_MODULE_PATH: "{{.G_CLP_FFI_JS_BUILD_DIR}}/ClpFfiJs-node.js" | ||||||||||||||||||||||
| G_CLP_FFI_JS_WORKER_MODULE_PATH: "{{.G_CLP_FFI_JS_BUILD_DIR}}/ClpFfiJs-worker.js" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| G_CLP_FFI_JS_CHECKSUM_FILE: "{{.G_BUILD_DIR}}/clp-ffi-js.md5" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # NOTE: clp-ffi-js normally builds its WASM artifacts against a pinned CLP source revision. For | ||||||||||||||||||||||
| # local CLP development and integration testing, we replace that dependency with the current | ||||||||||||||||||||||
| # workspace CLP checkout via a symlink. | ||||||||||||||||||||||
| # | ||||||||||||||||||||||
| # This allows us to quickly verify whether in-flight CLP changes remain compatible with downstream | ||||||||||||||||||||||
| # clp-ffi-js while reusing the existing clp-ffi-js build and test workflow through a small set of | ||||||||||||||||||||||
| # hacks/overrides. | ||||||||||||||||||||||
| # | ||||||||||||||||||||||
| # Run `task:clp-ffi-js` once to perform the initial setup. After that, selectively run | ||||||||||||||||||||||
| # - `task:clp-ffi-js:build-and-test-node` | ||||||||||||||||||||||
| # - `task:clp-ffi-js:build-and-test-browser` | ||||||||||||||||||||||
| # as needed. Note that `sudo` access is required for the latter. | ||||||||||||||||||||||
Bill-hbrhbr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||
| tasks: | ||||||||||||||||||||||
| default: | ||||||||||||||||||||||
| dir: "{{.G_CLP_FFI_JS_SRC_DIR}}" | ||||||||||||||||||||||
| deps: ["download-src"] | ||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||
| # Expose `clp-ffi-js` tasks for test setup | ||||||||||||||||||||||
| - task: "symlink-yscope-dev-utils" | ||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we make inline? Might be cleaner to make both inline. |
||||||||||||||||||||||
| - |- | ||||||||||||||||||||||
| sed -i 's/^\(\s*internal:\s*\)true\b/\1false/' taskfile.yaml | ||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given the tasks in clp-ffi-js is now publicly (external to the clp-ffi-js) used, maybe we should change the tasks in clp-ffi-js to non-internal? so long we dont document those tasks in clp-ffi-js docs, it shouldn't cause confusion to developers about what tasks they should run? if you really want, i think we can add inline docstrings to those tasks to explain thse are needed to set up the right dependencies for integration tests in the clp project. either way, it should be cleaner / more future-proof than this sed here |
||||||||||||||||||||||
| task node-modules | ||||||||||||||||||||||
| task config-cmake-project | ||||||||||||||||||||||
| rm -rf build/deps/clp | ||||||||||||||||||||||
| ln -s {{.ROOT_DIR}} build/deps/clp | ||||||||||||||||||||||
Bill-hbrhbr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Repeatable test workflow | ||||||||||||||||||||||
| - task: "build-and-test-node" | ||||||||||||||||||||||
| - task: "build-and-test-browser" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| build-and-test-node: | ||||||||||||||||||||||
| dir: "{{.G_CLP_FFI_JS_SRC_DIR}}" | ||||||||||||||||||||||
| deps: ["build-targets"] | ||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||
| - |- | ||||||||||||||||||||||
| VITE_NODE_MODULE_ABS_PATH="{{.G_CLP_FFI_JS_NODE_MODULE_PATH}}" \ | ||||||||||||||||||||||
| npm run test -- --project node | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # NOTE: Browser tests require installing browsers and system dependencies. The task is skipped if | ||||||||||||||||||||||
| # sudo is unavailable. | ||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| build-and-test-browser: | ||||||||||||||||||||||
| dir: "{{.G_CLP_FFI_JS_SRC_DIR}}" | ||||||||||||||||||||||
| deps: ["build-targets"] | ||||||||||||||||||||||
| status: | ||||||||||||||||||||||
| - "! (command -v sudo >/dev/null && sudo -n true)" | ||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||
| - |- | ||||||||||||||||||||||
|
Comment on lines
+57
to
+60
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| npm run test:init | ||||||||||||||||||||||
| VITE_WORKER_MODULE_ABS_PATH="{{.G_CLP_FFI_JS_WORKER_MODULE_PATH}}" \ | ||||||||||||||||||||||
| npm run test -- --project browser | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Builds clp-ffi-js + CLP with Emscripten + Clang | ||||||||||||||||||||||
| build-targets: | ||||||||||||||||||||||
| internal: true | ||||||||||||||||||||||
| run: "once" | ||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||
| - |- | ||||||||||||||||||||||
| cmake \ | ||||||||||||||||||||||
| --build "{{.G_CLP_FFI_JS_BUILD_DIR}}" \ | ||||||||||||||||||||||
| --parallel \ | ||||||||||||||||||||||
| --target "ClpFfiJs-node" "ClpFfiJs-worker" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| download-src: | ||||||||||||||||||||||
| internal: true | ||||||||||||||||||||||
| deps: | ||||||||||||||||||||||
| - "::init" | ||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||
| - task: "yscope-dev-utils:remote:download-and-extract-tar" | ||||||||||||||||||||||
| vars: | ||||||||||||||||||||||
| CHECKSUM_FILE: "{{.G_CLP_FFI_JS_CHECKSUM_FILE}}" | ||||||||||||||||||||||
| FILE_SHA256: "5ab1c27031caafb014198d4db711a71b3fbb1661b396bcd4e46764194e2b1f61" | ||||||||||||||||||||||
| OUTPUT_DIR: "{{.G_BUILD_DIR}}/clp-ffi-js" | ||||||||||||||||||||||
Bill-hbrhbr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||
| TAR_FILE: "{{.G_BUILD_DIR}}/clp-ffi-js.tar.gz" | ||||||||||||||||||||||
| URL: "https://github.com/y-scope/clp-ffi-js/archive/4cc6a7c.tar.gz" | ||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you think it's worth adding a docstring to explain when this commit was available? / in the future if we pin those commit ids at specific clp-ffi-js releases, it's better to comment with the release versions as well
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. something like |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| symlink-yscope-dev-utils: | ||||||||||||||||||||||
| internal: true | ||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||
| - |- | ||||||||||||||||||||||
| rm -rf "{{.G_CLP_FFI_JS_SRC_DIR}}/tools/yscope-dev-utils" | ||||||||||||||||||||||
| ln -s "{{.ROOT_DIR}}/tools/yscope-dev-utils" \ | ||||||||||||||||||||||
| "{{.G_CLP_FFI_JS_SRC_DIR}}/tools/yscope-dev-utils" | ||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if there's no easy way to set up
clp-ffi-jsas one ofdepsforbuild-and-test-nodeandbuild-and-test-browser, how do you feel about adding this to both tasks: