search

xyncro / freya

Freya Web Stack - Meta-Package
https://freya.io
Other
329 stars 30 forks source link

Added failing tests for conditional requests #178

Closed Vidarls closed 8 years ago

Vidarls commented 8 years ago

ref #176 and #175

kolektiv commented 8 years ago

This is great! Thanks for making this easy to work with, I like the approach as well, I'll try and expand it.

Vidarls commented 8 years ago

I usually find it easier to work with complex stuff like this with a good test suite :-), easiest way to get a repeatable repro :)

BUT: As mentioned, I am not an http guru, you probably should validate the premise of my tests, they are very naive and covers only this particular case.

kolektiv commented 8 years ago

Oh it's definitely easier :) I'll validate against my understanding (and the well-work RFCs piled up around me) and take it from there!

Vidarls commented 8 years ago

Made small change that made the tests pass on my machine.

I can not claim I really understand why my change worked, just came to the conclusion by debugging the faulty tests. Please review carefully.

Vidarls commented 8 years ago

I put the tests back into failing when no content is received. I'd be happy to hack away myself, but I would need some pointers on how I should go about understanding the inner workings of Freya in order do be of any help. Currently I am not even able to find where some decisions causes some handlers to fire.

kolektiv commented 8 years ago

I've got some time later on today set aside to try and iron these out. At the moment the internals of Freya are not the easiest places to understand, and the future direction of Freya will simplify them in various ways. With the tests you've provided tracking this down shouldn't be too bad - they're very useful indeed, so thank you!

Vidarls commented 8 years ago

Much appreciated.

As a side note: It might be worth the time to do some serious docs on how Freya works in order to help people who wants to start contributing.

I find it hugely interesting, but also slightly intimidating, and It does not feel good that me contributing takes more of your time than you just fixing it yourself.

Vidarls commented 8 years ago

docs might be "go read this piece on graph based machines"

kolektiv commented 8 years ago

Yes I agree with you on the internals. The only reason I'm slightly reluctant to spend too much time on docs for machine internals is that the internals for machines in vNext (4.0) are significantly changed (improved), so although the outward surface shouldn't change, the implementation will be different.

(Developer experience should be much the same but with a more powerful engine underneath).

Docs on the internals of other elements of Freya, and the implementation in general, are probably needed though to make contributing easier.

kolektiv commented 8 years ago

Hello! I think there was actually a wrong line in one of the tests which was making it look like things were failing, but in actual fact were displaying the correct behaviour. All of your tests now pass, and I think they're right - but if you don't agree, tell me! That should have produced a new set of pre-release packages.