Skip to content
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

add a "test" mode #30

Closed
wants to merge 8 commits into from
Closed

Conversation

nvdk
Copy link
Member

@nvdk nvdk commented May 27, 2021

this mode works differently from dev in that it creates a merged package.json file, so we have both the dependencies of the template and of the app available. In case of overlapping keys the values from the app are preferred over those from the template. This is typically a requirement to successfully run tests, and may also be considered as a sane approach for the dev script.

Merging of the package.json files is done using jq's multiply function and probably has some limitations I'm currently not aware of. So far it seems to be working.

this mode works differently from dev in that it creates a merged package.json
file, so we have both the dependencies of the template and of the app available
in case of overlapping keys the values from the app are preferred over those
from the template. This is typically a requirement to successfully run tests.

Merging is done using jq's multiply function and probably has some limitations I'm
currently not aware of. So far it seems to be working.
Dockerfile Outdated Show resolved Hide resolved
@@ -3,8 +3,11 @@ then
# Run live-reload development
exec /usr/src/app/node_modules/.bin/nodemon \
--watch /app \
--ext js,mjs,cjs,json \
--ext js,mjs,cjs,json,hbs \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really related to this PR, but reload on changed hbs files is appreciated

run-test.sh Outdated
# add some required files from the template
cp -a /usr/src/app/.babelrc.js /usr/src/app/helpers /usr/src/out/
# merge package.json files, prefering whatever is specified in the app
jq -s '.[0] * .[1]' /usr/src/app/package.json /app/package.json > /usr/src/out/package.json
Copy link
Member Author

@nvdk nvdk May 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-s is for slurp, which means read the entire thing before applying the operation
* is the multiplier operation, documented as:

Multiplying two objects will merge them recursively: this works like addition but if both objects contain a value for the same key, and the values are objects, the two are merged with the same strategy.

more info on https://stedolan.github.io/jq/manual/#Builtinoperatorsandfunctions

@nvdk
Copy link
Member Author

nvdk commented Jun 2, 2021

this still has an issue where the shell doesn't quit after running the tests, haven't figured out why yet. this has been solved

nvdk added 2 commits June 3, 2021 13:21
since v4 To avoid false positives and encourage better testing practices, Mocha
will no longer automatically kill itself via process.exit() when it thinks it
should be done running.

While probably somewhat desirable behaviour during development, this is
definitely not wanted in a ci setting. for now forcing the old behaviour to
exit. this should probably be a flag though

see https://boneskull.com/mocha-v4-nears-release/#mochawontforceexit
@madnificent
Copy link
Member

Hi @nvdk. I would like to let this issue blow up :-)

This seems to be strongly related to the dev setup issue you have with the 'mu' package in this package as well as in the mu-semtech/mu-ruby-template package. Your aim was to have a dev setup that would also be installable by regular scripts.

My understanding is that a published package would help solve some issues. With the slowly evolving branding to semantic.works, we could make a shift to published packages perhaps. A first ideation leads to publishing a 'semantic-works' npm package from which the current imports can happen (query, app, sparqlEscapeString, ...). The template may depend on more than only that package, hence we could publish a package that contains all dependencies in lockstep with the template's version.

Such approach would mean we can execute this merge on all environments. For backwards compatibility, we could keep supporting the current mu imports as well, though we might require an upgrade if you want to use this tests feature.

Does this approach sound reasonable? Is ticket more urgent right now? If you expect further discussion on this topic specifically, I suggest we move the discussion into a separate issue on mu-semtech/mu-project.

@nvdk
Copy link
Member Author

nvdk commented Jul 8, 2021

We're currently running on a fork of the javascript template that has this feature, so there is no urgency anymore. I do think you could still see things separately (that is if the template should offer testing capabilities). I agree the discussion on the future templates would fit better as an issue mu-project.

@erikap
Copy link
Member

erikap commented Aug 8, 2022

Replaced by #29, #43 and #44 .

@erikap erikap closed this Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants