Phpunit overhaul

[DaSourcerer]

Did I say this were an ongoing effort? Certainly is. Still far from complete, but there are now 196 assertions in 87 tests and counting. Coverage is somewhere in the 90% range. Not that this means anything: Some tests are still close to being pointless. But overall, the test suite is in quite okay shape.

[DaSourcerer]

I worked a bit on integrating the coveralls.io service during the weekend, whcih should've went into its own branch in retrospect … anyways, currently it would allow for this shiny new badge:

I also took the opportunity to have the code checked though codeclimate:

[Kubo2]

@DaSourcerer Isn't it better to rebase instead of creating merge commits?

[DaSourcerer]

@Kubo2: Probably yes. The process of rebasing always scares me a bit as it's not entirely unproblematic.

[DaSourcerer]

Ok, I'm pretty much done: There are now 250 assertions in 111 tests. The tests for HTMLSafeVisitor and NestLimitVisitor could still need some work but to be frank, I'm out of motivation right now.

@jbowens: Are you ok with that coveralls.io thing? If not, I can still remove it. TBH, it wasn't that trivial to get it working either.

[DaSourcerer]

@jbowens: Would it help if I condense everything into a single commit? Or are you uncomfortable with coveralls.io?

[jbowens]

@DaSourcerer yeah, please squash your commits

[jbowens]

?

[DaSourcerer]

Oh that, yes … must've slipped through. It appears an empty ElementNode is not rendering into an empty string as it expects to have a tagName set, so you're getting something like [][/] here. The proposed change in #56 would somewhat take care of that.

[DaSourcerer]

k, will do as soon as I get home. You're still cool with the coveralls-thing? You would need to sign up and set two environmental variables in Travis to get it to run.

[DaSourcerer]

Heh, looks like the latest version of my IDE came with an important new feature: Havoc-free interactive rebases \o/

[Kubo2]

@DaSourcerer That's interesting (and getting even better with history :-). Any news on this?

[DaSourcerer]

@Kubo2 What are you missing? Most of what isn't covered by test cases atm is deprecated, only leaving room for improvements within the tests for HTMLSafeValidator and NetLimitValidator. The coveralls-thing could need some spit and polish, though as it's going to fail spectacularly in pull requests (this is due to a security restriction within Travis CI). Oh, and it seems hhvm-nightly is no longer available on Travis.

Just to have the setup for coveralls.io documented:

1. Sign up with coveralls.io (can be done with GitHub auth)

2. add the jBBCode repository

3. Copy the repo token (atm that's at the right hand side of the repository view)

4. Go to the repository settings in Travis CI and add two new environmental variables:

   • COVERALLS_REPO_TOKEN set to the repo token from step 3
   • COVERALLS_RUN_LOCALLY set to 1

   Double check COVERALLS_REPO_TOKEN is hidden during builds. That's actually the default, but since this is a shared secret, it can't hurt to be a bit paranoid.

5. All done 😄

[DaSourcerer]

@Kubo2 @jbowens Just removed hhvm-nightly. Everything else should be good to go. As is, coveralls will fail but it won't taint the build itself (see the last build)

[Kubo2]

Hey @DaSourcerer, when #61 is merged, you would probably like to add some more tests for CodeDefinitionBuilder and CallableValidatorAdapter on top of it directly in this PR.

[DaSourcerer]

@Kubo2: I'd rather see it PR-authors were implementing their own tests suiting their changes. This PR is really an overhaul of the test suite to get it into better shape; it's not an effort by me to spare others from writing meaningful tests. Besides, with no feedback from @jbowens since about two months, I'm reluctant to invest any more time in this for the time being.

[Kubo2]

Oh, okay. I will add the tests.

[shmax]

@jbowens @Kubo2 Are you guys still planning on merging this?

[Kubo2]

I'd be pleased to see it merged, however I am getting worried about the bus
factor of the project. I haven't heard about @jbowens for quite a long
time. Moreover, my #61 kinda depends on this too.

Regards,
Kubis
On 06-Apr-2016 9:41 pm, "Max Loeb" notifications@github.com wrote:

@jbowens https://github.com/jbowens @Kubo2 https://github.com/Kubo2
Are you guys still planning on merging this?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#53 (comment)

[DaSourcerer]

@Kubo2, @shmax: I actually were to post something quite in that spirit. It's been now over a year since this PR opened and we are closing in on an entire year since the last commit to master which is leaving me equally worried.

I still believe in the technical merits of this project and would love to continue working on it. In fact, I am very much looking forward to replace my own regexp-based parser with jBBCode in one of my projects. However, in order to do so, I need text nodes with way more powerful capabilities. I see the need for some code reorg to do this cleanly, so I always considered this PR to be a blocker as the increased test coverage would allow for more confidence when implementing the needed changes.

Having said this, I am increasingly concerned over the lack of participation by @jbowens, leaving me (and not only me) with the impression of an abandoned project. This is a situation that would usually suggest forking jBBCode; yet I cannot state adequately just how much I would hate doing so: I'd much rather contribute to an existing project than create and maintain my own.

[shmax]

@jbowens come back to us!

[shmax]

@jbowens if you're no longer interested in maintaining this project, may I suggest giving @DaSourcerer write permissions?

[Art4]

@jbowens I am very sad to see this project abandoned, especially because there are some active supporter. If you don't have much time I would recommend to give someone write permission, like @shmax mentioned.

[jbowens]

Please squash all your commits. I'll try to give this another look tonight or tomorrow.

[DaSourcerer]

[x] done. Sorry for the holdup.