Unit tests about typo3-realurl HOT 8 CLOSED

Not test driven. RealURL is about Frontend, database and TypoScript, all are complicated to fake with TYPO3 and Backend. Unit testing is planned but no specific time yet. If you can help with implementation, you are more than welcome :)

Possibly we should do it in another ext (realurl_tests) to make a smaller extension.

from typo3-realurl.

Glad to hear that! I'll try to contribute but currently I can't tell when I'll find time for this. For integration tests maybe functional testing can help.

Regarding the possibility of a separate extension for testing: As far as I know (please correct me if I'm wrong) the Test folder has no impact on the performance of a production environment except of the waste of disk space. Thus I would prefer compliance with the conventions. With composer --prefer-dist and .gitattributes (see http://stackoverflow.com/questions/17049313/how-to-ignore-directories-with-composer) you're also able to exclude folders for production environments.

from typo3-realurl.

I've found a little bit time for a first overlook. The main problem is currently that the decoders and encoders missing decomposition. Thus we have two huge and hard to read monoliths lacking a public API. So I would recommend to decompose these both candidates at first in three steps:

1. Encapsulate URLs in a (mutable) class or two (mutable/immutable).
2. Move all useful helper methods from the encoder/decoder into suitable utility classes.
3. Design and implement adequate classes and interfaces to decompose the segment processing (preVar, fixedPostVar, postVar).

For the first step I could make a proposal (including unit tests) if you want.

from typo3-realurl.

Let me comment :)

Encapsulate URLs

I am not sure how this will help. URL is just a string. May be, you can tell more about the idea.

Move helper methods

Common methods are already extracted to a base class (EncodeDecodeBase). Other methods are quite integrated: they use instance variables to do their work. I'd prefer to keep the classes focused. Tests are suplemental, they should not complicate the code or introduce obstacles.

Segment processing

All xxxVars are handled by the same code. I do not see what can be extracted here.

Other thoughts

To me the best way to make tests would be:

• Be able to test RealURL as a whole: pass a source URL to it and test that the result matches.
• Be able to test certain key functions (like path encoding/decoding or xxxVar encoding/decoding). This will be extremely useful exclusion from path is implemented.

In general I feel that unit testing would be complicated. RealURL requires rootline in many cases. Probably, that can be extracted first. Than tests would be able to give a fake rootline and provide stable results for all tests. Test rootline provider can return all kind of rootlines are required by tests (for example, with mount points or with excluded segments).

I think we should be really careful when we implement tests. The main goal of RealURL is "real life" encoding and decoding, which should be fast and reliable. If tests somehow screw up tests, it would not be good.

from typo3-realurl.

Encapsulate URLs
Currently you have a lot of code which operates on URL-related data. This is code might be a little bit more faster but it is not DRY and thus brings more potential bug sources, makes the code hard to read an maintain; ok you are mostly the maintainer here :).

URL is just a string.

Well it is, but to be more precisely it's a syntax, thus you need code to read and manipulate this syntax and this code is currently ;) ... see above. If you really want unit tests you need a public interface what you can test. Your current decoder has two public methods, so its no wonder when you write:

In general I feel that unit testing would be complicated.

With such a monolith it would be more complicated and it's less unit testing than more integration testing.

In case of the encoder you have some more public methods (postProcessEncodedUrl and cleanUpAlias) but beside encodeUrl I see no effort having them public in this module.

Don't get me wrong here the encoder/decoder interface with encodeUrl() and decodeUrl() is fine in the first place. But currently you have put a procedural solution in a class and give them a "main" method. What's missing is decomposition! You have already done that with the caching aspect ;). But your code related to the URL processing currently relies heavily on low level string and array operations.

Move helper methods

Common methods are already extracted to a base class (EncodeDecodeBase).

I'm talking about helper methods but I'm pretty sure I get your habits here:

I'd prefer to keep the classes focused. Tests are suplemental, they should not complicate the code or introduce obstacles.

And here we have the same problem, all your so called "common methods" are not part of the public interface it's obvious why; even the caching aspect is not completely decoupled from these monoliths.

So no unit tests where no units are. I would call it more a lack of software design and architecture. But if you "feel" that way I should should stop here because I don't want to argue with the programming habits of the maintainer or the effort of OO, which would be silly. So maybe I must hope that the core brings a better designed solution if they not decide to take your current solution... I don't know :).

from typo3-realurl.

You forget that RealURL operates as a set of hooks :) Those hooks are in the TYPO3 core since TYPO3 3.x and it is unlikely that they will ever change. They dictate a certain approach and certain public methods.

postProcessEncodedUrl is a hook functions, so it has to be public. Same for encodeUrl and decodeUrl. It just cannot be done the other way because TYPO3 requires such approach.

But your code related to the URL processing currently relies heavily on low level string and array operations.
Well, I like a balanced approach :) One one hand I like to extract things to meaningful classes when it makes sense. One the other hand, if it creates too much overhead, I avoid it. For me it is important to do what makes sense, not what is theoretically correct. Otherwise I could end up in something like this: https://github.com/Herzult/SimplePHPEasyPlus (creating a special class for simple integer addition). This is a good example about the theory being right but making no sense.

if you "feel" that way I should should stop here because I don't want to argue with the programming habits of the maintainer or the effort of OO, which would be silly
No, I do not think so. I think the current code is quite well structured for testing. For example, it is possible to test all xxxVars methods (except one) because they only use internal class data. How to make it? It remains to be seen.

I would like to have unit tests but the main goal of this code is to be fast and efficient in its main goal: encoding and decoding URLs. If that can be combined with unit testing, I am all in :)

I used unit tests with Java and non-TYPO3 PHP software and it was quite easy. With TYPO3 the main problem for me is the code reliance on the database and global objects. I prepare for this by using instance attributes. However the rest still remains to be seen.

So maybe I must hope that the core brings a better designed solution if they not decide to take your current solution
RealURL is a hack because the current TYPO3 core is not meant to use speaking URLs. Regardless of how the code is written and who makes, it will be always a hack with the current TYPO3 architecture. However, if proper routing is integrated in the TYPO3 core, things may change.

from typo3-realurl.

You forget that RealURL operates as a set of hooks :) Those hooks are in the TYPO3 core since TYPO3 3.x and it is unlikely that they will ever change. They dictate a certain approach and certain public methods.

No I don't. :P

postProcessEncodedUrl is a hook functions, so it has to be public. Same for encodeUrl and decodeUrl. It just cannot be done the other way because TYPO3 requires such approach.

Regarding postProcessEncodedUrl I was in the hurry, please excuse. I typically split these things up in TYPO3 CMS projects: A handler for hooks is placed in the namespace Hook which would use the UrlDecoder respectively UrlEncoder, so this is the reason why I mixed it up. My mistake, sorry :).

Well, I like a balanced approach :) One one hand I like to extract things to meaningful classes when it makes sense. One the other hand, if it creates too much overhead, I avoid it. For me it is important to do what makes sense, not what is theoretically correct. Otherwise I could end up in something like this: https://github.com/Herzult/SimplePHPEasyPlus (creating a special class for simple integer addition). This is a good example about the theory being right but making no sense.

I agree, I see it the same way! Except for decimal calculations in financial applications https://github.com/Herzult/SimplePHPEasyPlus seems like a joke project?

No, I do not think so. I think the current code is quite well structured for testing. For example, it is possible to test all xxxVars methods (except one) because they only use internal class data. How to make it? It remains to be seen.

Unfortunately the encoder and decoder are not well structured for testing. That's my point here! Except the constructor and the event handler (hooks) all methods are not part of the public interface and thus not meant to be tested with unit tests. And for that you need to decompose these monoliths before. Then you're able to test a lot of code without thinking about the database and the TypoScript stuff (which can however be mocked, but I would do that after decomposition your monoliths).

As long as we can't behold important aspects of your extension isolated forget unit tests. One of these aspects is the processing of URLs (parsing, manipulating etc.). But currently that's all tightly coupled with application code. Another aspect which seems to be much more "well structured" for testing is caching.

I would like to have unit tests but the main goal of this code is to be fast and efficient in its main goal: encoding and decoding URLs. If that can be combined with unit testing, I am all in :)

I think it depends on how you would define "fast and efficient". If this not mean "as fast as possible" and every additional copy of an array or every additional class is a huge problem I would say it is. :)

I used unit tests with Java and non-TYPO3 PHP software and it was quite easy. With TYPO3 the main problem for me is the code reliance on the database and global objects. I prepare for this by using instance attributes. However the rest still remains to be seen.

Same here at least for our Java projects; I think the reason why TDD is easier for Java applications is founded in the language itself and its well designed framework. E.g. I would not come up with the idea of using only Strings and the Collection Framework when I process URLs, at least I would use something like java.net.URL and javax.ws.rs.core.UriBuilder. And especially the URL path and the URL query having an important role in this project.

RealURL is a hack because the current TYPO3 core is not meant to use speaking URLs. Regardless of how the code is written and who makes, it will be always a hack with the current TYPO3 architecture. However, if proper routing is integrated in the TYPO3 core, things may change.

Yes I know that. But I think this doesn't mean its implementation have also to be a hack?

But I also see the risk when the core come with a well defined routing concept all work here might be obsolete :(. Do you know more about the current state of this feature? I've read about plans after 7.2 was released but haven't found more (e.g. concept papers).

from typo3-realurl.

ps: I really appreciate that you take the time for this discussion! 👍

from typo3-realurl.