You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
features/support/fbresult_generated.js is a generated file that is committed to the repository. #7707 adds npm run regenerate-fbresult so rebuilding it is one command, but it is still a manual step that someone has to remember after touching include/engine/api/flatbuffers/*.fbs.
It should be generated on demand as part of the test run instead, so the committed copy can be deleted.
Why it is worth doing
Forgetting the step does not fail loudly. The step definitions keep decoding the previous field layout, so an accessor like fb.table() silently reads whichever vector now occupies that vtable slot. The failure surfaces as a wrong matrix or a null result somewhere in features/step_definitions/, nowhere near the schema that actually changed.
This has already happened. In #7704 the schema gained a field and the bundle was not regenerated, and the mismatch was only caught by reading vtable offsets by hand.
The C++ side does not have this problem. CMakeLists.txt:336 runs flatbuffers_generate_headers at build time from the same schemas, so the headers cannot drift. Only the JavaScript bundle can.
Sketch
The pieces already exist:
flatc ships with the vcpkg flatbuffers port and is present in any configured build
So this is mostly a question of where to hook it. Options, roughly in order of appeal:
Generate into a gitignored path during cucumber setup, in features/support/ or a pretest npm script, and point the two importers at it. Closest to the CMake model.
Keep it a build artifact: have CMake emit the JavaScript bundle next to the C++ headers, since it already knows where flatc is.
Leave the file committed and add a CI check that regenerating produces no diff. Cheaper, but it catches the mistake rather than preventing it.
The two importers are features/step_definitions/distance_matrix.js and features/step_definitions/nearest.js.
Things to watch
The generation needs flatc, so a cucumber run would gain a dependency on a configured build or a flatc on PATH. That is already true in CI, and mostly true locally, but it would be a new failure mode for anyone running the features against a prebuilt binary.
Issue
features/support/fbresult_generated.jsis a generated file that is committed to the repository. #7707 addsnpm run regenerate-fbresultso rebuilding it is one command, but it is still a manual step that someone has to remember after touchinginclude/engine/api/flatbuffers/*.fbs.It should be generated on demand as part of the test run instead, so the committed copy can be deleted.
Why it is worth doing
Forgetting the step does not fail loudly. The step definitions keep decoding the previous field layout, so an accessor like
fb.table()silently reads whichever vector now occupies that vtable slot. The failure surfaces as a wrong matrix or a null result somewhere infeatures/step_definitions/, nowhere near the schema that actually changed.This has already happened. In #7704 the schema gained a field and the bundle was not regenerated, and the mismatch was only caught by reading vtable offsets by hand.
The C++ side does not have this problem.
CMakeLists.txt:336runsflatbuffers_generate_headersat build time from the same schemas, so the headers cannot drift. Only the JavaScript bundle can.Sketch
The pieces already exist:
flatcships with the vcpkg flatbuffers port and is present in any configured buildesbuildis a pinned devDependencyscripts/regenerate_fbresult_js.shfrom build: add a script to regenerate the flatbuffers JavaScript bundle #7707 does the work, and its output is deterministicSo this is mostly a question of where to hook it. Options, roughly in order of appeal:
features/support/or apretestnpm script, and point the two importers at it. Closest to the CMake model.flatcis.The two importers are
features/step_definitions/distance_matrix.jsandfeatures/step_definitions/nearest.js.Things to watch
flatc, so a cucumber run would gain a dependency on a configured build or aflatconPATH. That is already true in CI, and mostly true locally, but it would be a new failure mode for anyone running the features against a prebuilt binary.--gen-alland running esbuild from the work directory.Requirements / Relations
Follows #7707. Came out of #7704.