#192 Add fedmsg/markup route, and api tests
Merged by atelic. Opened by atelic.
atelic/fedora-hubs feature/unittest  into  unittest

Download 192.patch
  • Adds tests for all /api routes declared in app.py

  • Adds new api route for the live feed

My old __init__.py looked like this.

Any hints in the traceback about what raised the 500?

Oh that line is important. It allows this check to pass.

I tried to use my old init file and had to add self.nickname = username to FakeAuthorization. I caused errors in your tests though because I seem to be missing something that you added

I wonder how much of this we could/should do server side (SSE server I mean)

I would imagine that we can do the marking up on that end then use the flask server to filter out messages that the user doesn't have notifications set for.

We would then change this route name to something more fitting

1 new commit added

  • Add tests for /api/hub/*

Well, my idea is that hub does not filter out any message, FMN is the place where people set their preferences for notifications and I think it would be best to leave all the filtering to be done by FMN. So basically hubs would get a feed for a certain user and just display everything that's in there, no more reasoning on hubs' part :)

How does that sound?

finally got my machine back up :)
LGTM 👍
Ran 42 tests in 7.606s OK (SKIP=12)
57% coverage now :)

Glad you got it working! :thumbsup: So is this good to merge?

I think this implementation is much better. After reading this comment I think I have a better understanding of how this would work.

Do you think this is OK to merge for now and then make the modifications to this once FMN has that ability?

I'm ok with merging it, maybe we can just document that this is also not meant to stay?

2 new commits added

  • Add tests for /api/hub/*
  • Add fedmsg/markup route, and api tests

I don't think we need this function. auth_set is enough i believe since i dont think g.fas_user is ever used

It looks like g.fas_user is needed in order for some FMN magic to work. When switching the TestFedmsg methods to auth_set, they fail. Full trace here

it also seems that hubs never sets g.fas_user so manual testing will fail but the unittests will pass. I wonder if this has anything related to do with the authorization_layer missing?

g.fas_user is set by flask-fas-openid (part of python-fedora)

Will we be changing hubs to flask-fas-openid? it seems we are using from flask.ext.openid import OpenID

Oups sorry, you are correct, we're directly using flask-openid here

And a quick git grep fas_user returns nothing, so indeed, setting it here shouldn't be needed

Note that you may prefer to use some things like self.assertEqual() or self.assertIn() as what is returned in the log is then a little more explicit (it would tell you 200 != 400 for example)

Okay after fiddling with app contexts and Flask() vs FlaskClient() this should be working now.

2 new commits added

  • Add tests for /api/hub/*
  • Add fedmsg/markup route, and api tests

1 new commit added

  • Apply pep8 and move json data to their own files

Looks a little over-indented here

3 new commits added

  • Apply pep8 and move json data to their own files
  • Add tests for /api/hub/*
  • Add fedmsg/markup route, and api tests

3 new commits added

  • Apply pep8 and move json data to their own files
  • Add tests for /api/hub/*
  • Add fedmsg/markup route, and api tests

Ran 42 tests in 7.703s; OK (SKIP=12); 56% Coverage

lgtm :thumbsup:

Pull-Request has been merged by atelic

Metadata