phpcs-security-audit icon indicating copy to clipboard operation
phpcs-security-audit copied to clipboard

Add CI/build testing

Open jrfnl opened this issue 5 years ago • 6 comments

I'd like to propose adding basic build testing via Travis.

Things which could be included, would, for instance, be:

  • [x] Validation of the composer.json file. - PR #60
  • [ ] Dependency security check.
  • [x] Validation of the rulesets - including the example rulesets - against the PHPCS xsd schema. - PR #60
  • [x] Code style consistency check for the XML rulesets. - PR #60
  • [x] Linting of the PHP code against supported PHP versions. - PR #60
  • [ ] Code style consistency check. This would need a decision on what code style to use first.

In the future, once sniff specific tests have been added this could be expanded to also include:

  • [x] Unit test run. - PR #70
  • [ ] Code coverage check.
  • [ ] Sniff feature completeness check - is each sniff accompanied by unit tests and possibly documentation ?

Please let me know if you're interested in this and if so, if there are any other things you'd like to include or check you'd want to exclude.

jrfnl avatar Feb 18 '20 06:02 jrfnl

Those are really nice things to make this a mature project.

However, I can't commit myself on supporting this, and working on the current code to achieve good code quality.

Would you be interested into becoming a maintainer and be in charge of that part?

My skills are better applied on the security know-how and keeping things consistent within the tool than the actual PHP code itself. I'm sure it is technically poorly written at this point since most of code is around a decade old.

jmarcil avatar Feb 18 '20 08:02 jmarcil

@jmarcil These are easy enough to add for me.

Would you be interested into becoming a maintainer and be in charge of that part?

Well, I do kind of maintain a lot of standards already, but if it helps if I'm a co-maintainer to collaborate on this further, then: why not ?

Note: some caveats:

  • I can't commit to spending x amount of time on the project. I will contribute when I can.
  • I will not merge my own PRs.

Is that ok for you ?

This would need a decision on what code style to use first.

Have you got a strong preference for a particular kind of standard ? If not, I'd propose to use PHPCSDevCS which is a PSR-12 based standard specifically for PHPCS standards which are not aimed at code style (like the Security standard).

jrfnl avatar Feb 18 '20 09:02 jrfnl

PR #60 addresses action points 1, 3, 4 and 5.

jrfnl avatar Feb 18 '20 14:02 jrfnl

@jmarcil Just checking in: have you had a chance to look at my previous response above https://github.com/FloeDesignTechnologies/phpcs-security-audit/issues/56#issuecomment-587363408 ?

jrfnl avatar Mar 10 '20 10:03 jrfnl

Yes, but forgot to answer your questions.

It's OK, as long as I'm the sole person that can merge, I still consider myself alone in the endeavor of maintaining this tool.

So because of that, I will have to considerably diminish the amount of work being put on code quality and other refactoring of this tool. I've already spent more time this year on this than actual road-map items people requested since a long.

So with that in mind, I'd like to defer the code style decision and other nice to have in this current issue.

Is there anything else we absolutely need for potential unit tests to be able to run?

If not I think we should close this issue so I can refocus back on things that matter more for the usage of the tool versus the development of it.

Thank you for your understanding.

jmarcil avatar Mar 16 '20 01:03 jmarcil

So with that in mind, I'd like to defer the code style decision and other nice to have in this current issue.

I understand your choice to defer code-style work and will hold off from spending time on that.

Is there anything else we absolutely need for potential unit tests to be able to run?

PR #70 which I've just pulled would address the Unit test run bullet mentioned above.

Once that is merged, we could already consider adding Code Coverage checking as well.

After that, there's not that much remaining anyway.

If not I think we should close this issue so I can refocus back on things that matter more for the usage of the tool versus the development of it.

I'd prefer to keep the ticket open though until the points have all been addressed, even though they will be low priority and not get immediate focus for now.

jrfnl avatar Mar 16 '20 02:03 jrfnl