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

Tests are failing #1

Open
henri-hulski opened this issue Feb 22, 2017 · 5 comments
Open

Tests are failing #1

henri-hulski opened this issue Feb 22, 2017 · 5 comments

Comments

@henri-hulski
Copy link
Member

At the moment all test are failing with:

infos = [<morepath.predicate.PredicateInfo object at 0x7f478c85f7b8>]

 
    def toposorted(infos):
        """Sort infos topologically.
    
        Info object must have a ``key`` attribute, and ``before`` and ``after``
        attributes that returns a list of keys. You can use :class:`Info`.
        """
        key_to_info = {}
        depends = {}
        for info in infos:
            key_to_info[info.key] = info
            depends[info.key] = []
        for info in infos:
            for after in info.after:
>               after_info = key_to_info[after]
E               KeyError: <function request_method_predicate at 0x7f478cb77488>
 
../morepath/morepath/toposort.py:19: KeyError

@faassen Have you an idea what could be the issue?

@henri-hulski
Copy link
Member Author

henri-hulski commented Feb 28, 2017

After removing after=morepath.request_method_predicate from the predicate decorator, the error disappears and most tests are passing.
Though test_json_body_model and test_view_body_model_querytool are still failing.

@faassen Do you know what's going on?
Why the predicate after parameter is throwing an error here?

@henri-hulski
Copy link
Member Author

henri-hulski commented Feb 28, 2017

When printing out info.key, after and key_to_info in def toposorted(infos) from here, I get the following result:

info.key:
  <function model_predicate at 0x7fbdeab1d268>


info.key:
  <function name_predicate at 0x7fbdeab1d378>
after:
  <function model_predicate at 0x7fbdeab1d268>
key_to_info: {
  <function request_method_predicate at 0x7fbdeab1d510>: <morepath.predicate.PredicateInfo object at 0x7fbdea98cb00>,
  <function model_predicate at 0x7fbdeab1d268>: <morepath.predicate.PredicateInfo object at 0x7fbdea98ca90>,
  <function name_predicate at 0x7fbdeab1d378>: <morepath.predicate.PredicateInfo object at 0x7fbdea98cac8>
}


info.key:
  <function request_method_predicate at 0x7fbdeab1d510>
after:
  <function name_predicate at 0x7fbdeab1d378>
key_to_info: {
  <function request_method_predicate at 0x7fbdeab1d510>: <morepath.predicate.PredicateInfo object at 0x7fbdea98cb00>,
  <function model_predicate at 0x7fbdeab1d268>: <morepath.predicate.PredicateInfo object at 0x7fbdea98ca90>,
  <function name_predicate at 0x7fbdeab1d378>: <morepath.predicate.PredicateInfo object at 0x7fbdea98cac8>
}


info.key:
  <function body_model_predicate at 0x7fbdeab2e268>
after:
  <function request_method_predicate at 0x7fbdeab1d510>
key_to_info: {
  <function body_model_predicate at 0x7fbdeab2e268>: <morepath.predicate.PredicateInfo object at 0x7fbdea75b9b0>
}

So when checking for body_model_predicate the key_to_info dict contains only the body_model_predicate and no predicate from morepath so it can't look up request_method_predicate.

@faassen Is this a bug or am I missing something?

@henri-hulski
Copy link
Member Author

henri-hulski commented Feb 28, 2017

Aah it seems when using

@App.predicate(morepath.App.get_view, name='body_model', default=object,
               index=ClassIndex, after=morepath.request_method_predicate)
def body_model_predicate(self, obj, request):

instead of

@App.predicate(App.get_view, name='body_model', default=object,
               index=ClassIndex, after=morepath.request_method_predicate)
def body_model_predicate(self, obj, request):

it works (still the 2 tests are failing).

So I cannot use get.view from local App, I have to use it from morepath.App.
Is this intentional?

@faassen
Copy link
Member

faassen commented Mar 1, 2017

Good catch! That isn't intentional but it makes sense -- get_view has a different implementation for each subclass due to it being a reg dispatch_method.

What are the consequences of the first definition? Does it affect the superclass and make it use the body model predicate after all, or is it unrelated? I think they should still be unrelated but I'm not 100% sure off the top of my head. It would be good to create a test for this package that defines both a body model app and a normal app that derives from morepath.App, and then see that morepath.App doesn't suddenly get a body model or something.

Hm, this reminds me of something else that could go wrong: the request is overridden. This can only be done at the top level app, not at lower down apps. But we actually want to enable body model apps as sub-apps. So we'd need a test that mounts the body model app inside of a normal app, and I expect the test will fail. We'd like this to work but that would involve adding a feature.

What I think we need to do is add a feature to Morepath so that sub-apps can add new methods/properties to the common shared request. Or alternatively is cloning the request into a new request object when necessary if the sub app defines another request class.

The second approach would be cleaner as it separates concerns between requests, but there are some issues: what if a tween expects thing about requests? Tweens only work on the outside. What request class comes back then? I think the request class of the application currently in use. And it requires a reliable way to clone a Morepath request. webob request does expose a copy method, which we might be able to build on, though I'm not sure what it means by doing a shallow copy except for wsgi.input. Does it does a deep copy of that, or not a copy at all?

@henri-hulski
Copy link
Member Author

@faassen Have to look closer at your proposition.

The consequences if using local App.get_view for predicate are, that you can't use after and predicate_fallback doesn't work showing 404 instead of 422.

When using morepath.app.get_view only some querytool tests are failing when running from more.body_model:
test_view_body_model and test_dump_json are returning an empty array.
test_view also doesn't work.

Another issue:
The predicate documentation says:

You can use either self or request as the argument for the predicate function. Morepath sees this argument and sends in either the object instance or the request.

And in the example you use only the request argument. This seems to be wrong.
You need 3 arguments like used in core: def body_model_predicate(self, obj, request).

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

No branches or pull requests

2 participants