-
Notifications
You must be signed in to change notification settings - Fork 12
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
WebtoolsMapsContext has been duplicated, leaving actual used context untested #112
Comments
The reason is https://github.com/openeuropa/oe_webtools/pull/94/files#diff-b21fa0b75a72fac761e40d689e55338dR16 . |
Yes I know this, this change in BDE was my proposal 😁 But this is still in use in existing sites, so we should still keep it and ensure it is covered by tests until we are ready for OE Webtools 2.x |
Changing how we test our components does not constitute a change in the component's API and it does not motivates a new major release since sites will continue to work without any manual intervention. In this case tests will need to be ported to comply with the new structure. |
OK I agree that we shouldn't waste too much energy on this. Maybe a simple solution could be to add a simple check to ensure that both files contain identical code. Then we can keep the test coverage on the main context. We'll need to account for a few small differences like the module namespace and the disclaimer. I'll try to give this a shot. |
In #113 I have proposed a simple check to ensure that the contents of both contexts are identical. To account for the differences in the file headers (the docblocks and namespaces have some differences) I am only checking the lines inside the class. This is done by using a simple I have added this is a small shell script. I first tried to put it directly in I figured in the end that a shell script is probably the easiest solution, once we reach 2.x we can just remove the deprecated subcontext and the shell script and we're done. |
For some reason the
WebtoolsMapsContext
has been duplicated in #94, removing the test coverage that was in place for the actual subcontext that is in use in existing sites.This change was out of scope for that PR and is not motivated in any of the comments made. Let's revert this change to not disrupt test coverage for the existing subcontext.
It can be decided to step away from the subcontext in the module folder and move it to a context in the
tests/Behat
folder but since this affects current projects it can only be done in a major release.The text was updated successfully, but these errors were encountered: