-
Notifications
You must be signed in to change notification settings - Fork 1
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
Various modernisation of this package #2
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
84faf3f
to
d4850b7
Compare
d4850b7
to
fcc7660
Compare
bbe6a1e
to
ad3b26c
Compare
- Provide a new test registry specific for the "legacy" (fuller feature) AMD-based tests. Unfortuantely given that they are rather coupled to the features offered by RequireJS, these are now moved to their own registry. - Remaining tests should be usable from webpack.
- With the introduction of this feature in calmjs-3.4.0, the nunja advice can always be applied to ensure that the pre-built version of the template be always enabled for the webpack toolchain given that it isn't optimised for other forms of dynamic usage. This is in contrast to the RequireJS version where a more dynamic loading method may be kept enabled. - This also show that a particular test case that would not have been migrated before the introduction of this feature under the standard testing regime.
- This ensures that webpack will not actually try to load that RequireJS dynamically generated module that would never resolve (thus webpack will fail); this is done such that forcible execution of these tests through the webpack toolchain (by the manual invocation of this registry with that) would not fail prematurely. - Loading of this module via a variable would treat this as a dynamic load point and it exploits the fact that webpack can't try to resolve and fail on this.
- Renamed the nunja.mold.tests registry to nunja.mold.testing to signify that it isn't for as a provider for tests, but more for providing test molds that may be used for testing. This also means that dependents will no longer pick this up, as the standard runner will no longer automatically include these molds. - This allow a much finer control on how tests are run under the two currently implemented artifact bundlers. - Changes made also ensure that all forms are executed through the standard calmjs runner (artifact runner to follow), while the full coverage is still unfortunately dependant on the async template loader given how it was originall implemented. - Naturally, how those tests are skipped when conditions are unmet are also modified: - Skipped if the nunja.mold.testing registry isn't made available. - JavaScript testing frameworks just make this stupidly difficult, but returning a function that might or might not skip the test based on the condition works. - Also this is implemented in a way that allow the entire block to be excluded from coverage. - Given the huge number of tests to run, skip the artifact generation with the flag introduced in calmjs.dev-2.3.0.
ad3b26c
to
f2d5227
Compare
- Ensure that requirejs is not a hard requirement when trying to query for an available template (such that this operation will not trigger an exception when built without it).
f2d5227
to
d588932
Compare
- Given the number of test cases across different test modules have the same skip conditions (i.e. when the required registry is missing), put them together where it make sense.
- Not sure if it's Python 2.7 or Node.js 8 causing issue on the build, but if we can rule out Node.js, Python 2 support for Nunja will be dropped on Windows.
- Adopting one of the previous prototypes but name them explicitly as to which module system they support, with the RequireJS being labeled as such and only assigned to NunjaLoader if it is available.
9c99027
to
6c8b8f9
Compare
- Ensure that the appropriate things are included/excluded per default for each of the advice sets for toolchains.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a work in progress for this already very much work in progress prototype package. This is what happens when a JavaScript codebase is ignored for a couple years. Python doesn't have anywhere close to this level of integration issues because it doesn't have the myriad of barely working choices.