Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated Testing for e107 #4038

Merged
merged 383 commits into from
Dec 28, 2019
Merged

Automated Testing for e107 #4038

merged 383 commits into from
Dec 28, 2019

Conversation

Deltik
Copy link
Member

@Deltik Deltik commented Dec 4, 2019

Motivation and Context

Automated testing makes it possible to write code and verify the code's behavior without needing to check the output via a web browser. It also catches regressions in case a change to existing code causes tests to fail.

e107 had automated tests since 12 February 2018, but the tests were in a separate repository that loaded e107's source code from a Git submodule. This had various drawbacks:

  • The submodule could not be updated until code in e107 had already changed. This promotes a bad practice of writing tests after the new code had been written.
  • The submodule needed to be updated manually, so it was often left behind, which meant that the tests diverged from the actual code in use.
  • There was no link between the e107 repository and the e107-test repository, so other contributors didn't know about e107-test.

This pull request brings in the entirety of the e107-test repository into the main e107 repository under the folder e107_tests, in accordance with the long-established e107 folder structure.

Continuous integration with GitHub Actions is also implemented, and code coverage reports are uploaded to Code Climate.

Description

  • Moved tests from https://github.com/e107inc/e107-test to a folder called e107_tests in the main repository
  • Added GitHub issue templates for:
    • bug reports, and
    • feature requests
  • Added a GitHub pull request template. This post you are reading now is an example of the template filled in.
  • Added code coverage reports hosted on Code Climate
  • Continuous integration with GitHub Actions. Also features testing with multiple PHP versions and multiple MySQL/MariaDB versions.
  • Updated README.md with improved grammar and formatting
  • Fixed bugs in the e107 core caught by the tests:
  • Added an exclusion of the e107_tests folder for those updating e107 to the latest master commit via the web admin interface
  • Bonus: A SaltStack state file to deploy an e107 development and testing environment on an LXC container.

How Has This Been Tested?

Through the tests in the e107_tests folder!

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist

CaMer0n and others added 30 commits September 11, 2018 14:16
Now works with the best of both worlds:

* Barebones cleanup in slow Windows environments
* Git snapshots in other Git environments
InstallCest now drops all tables from the test database before running
each test.

Fixes: #13
To follow @myovchev's convention like eHelper

Per @CaMer0n
GitPreparer now registers a "priority" register_shutdown_function
callback in order to clean up in case of a fatal error.
Removed unused includes/bootstrap.php

Made a new e107 sample database dump
Added a guard to GitPreparer::unsetVcsInProgress() to prevent doing a
`git reset` when there are no test locks present.

Otherwise, the uncommitted changes in the app will be removed by the
shutdown feature introduced in 952c6e5.
e107pluginTest::testXmlSiteLinks() ignores the primary key and model
order because another test could have inserted records before this test.
Other tests have been meddling with the e107::wysiwyg() global state

e_parseTest::testToForm() now considers two outcomes of the
e107::wysiwyg() state.
e_shortcode_parser normally doesn't need reloading in an e107
installation because installing the "banner" plugin and parsing
shortcodes have always been two separate script calls (page loads).

It would slow down the e107 core to add an e_shortcode_parser reloader
after installing a plugin when the page would later exit without
parsing any shortcodes.
New test forces a failover of the `readfile()` internal function to
test the failover functionality of \e107\Shims\Internal::readfile()
New tests cover the behavior described in
#3547
The "stash" lock was used to shove .gitignore'd files under the rug
so that they would not interfere with a pure copy of the app.

Vendor files may be ignored in the app, so for performance, the
"stash" lock has been deactivated. Vendor files no longer need to
be downloaded each time the test runs.

The "commit" lock now includes all ignored files so that tests
are run with the files as they are.
- MOD: PHPDoc for e_file::unzipGithubArchive()
- NEW: e_file::unzipGithubArchive(): Added exclusions for files that don't exist in production
- NEW: e_file::unzipGithubArchive(): Accept a destination path argument for a custom extraction location
- NEW: Restored unimplemented skipped list in e_file::unzipGithubArchive()
- FIX: e_file::unzipGithubArchive(): Extraction fails if parent directory of file doesn't exist
- MOD: Type hint for Base::$deployer
- NEW: Basic test for e_file::unzipGithubArchive()
@Deltik Deltik requested review from Moc and CaMer0n December 27, 2019 18:02
@Deltik Deltik added core: testing framework type: enhancement An improvement or new feature request labels Dec 27, 2019
@Deltik Deltik changed the title [WIP] Add e107_tests Automated Testing for e107 Dec 27, 2019
@Deltik Deltik marked this pull request as ready for review December 27, 2019 18:20
@Deltik Deltik self-assigned this Dec 27, 2019
Error message from Code Climate:

> Exception: The given file "/code/e107_tests/lib/ci/salt/pillars/config-local.sls" does not exist. in /usr/src/app/vendor/pdepend/pdepend/src/main/php/PDepend/Engine.php
> #0 /usr/src/app/vendor/phpmd/phpmd/src/main/php/PHPMD/ParserFactory.php(129): PDepend\Engine->addFile('/code/e107_test...')
> #1 /usr/src/app/vendor/phpmd/phpmd/src/main/php/PHPMD/ParserFactory.php(109): PHPMD\ParserFactory->initInput(Object(PDepend\Engine), Object(PHPMD\PHPMD))
> #2 /usr/src/app/vendor/phpmd/phpmd/src/main/php/PHPMD/ParserFactory.php(76): PHPMD\ParserFactory->init(Object(PDepend\Engine), Object(PHPMD\PHPMD))
> #3 /usr/src/app/vendor/phpmd/phpmd/src/main/php/PHPMD/PHPMD.php(215): PHPMD\ParserFactory->create(Object(PHPMD\PHPMD))
> #4 /usr/src/app/Runner.php(116): PHPMD\PHPMD->processFiles('/code/e107_test...', 'cleancode,unuse...', Array, Object(PHPMD\RuleSetFactory))
> #5 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1852): CodeClimate\PHPMD\Runner->run(Array)
> #6 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1772): fork_daemon->invoke_callback(Array, Array, false)
> #7 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1673): fork_daemon->fork_work_unit(Array, '', -1)
> #8 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1445): fork_daemon->process_work_unit(-1)
> #9 /usr/src/app/Runner.php(31): fork_daemon->process_work(false)
> #10 /usr/src/app/engine.php(35): CodeClimate\PHPMD\Runner->queueDirectory('/code')
> #11 {main}
Increased possible random strings for unique fields in e_db_pdo::copyRow() from 1000 to 59^11 (UserHandler::generateRandomString() "alphanumeric" should have 59 characters to choose from)

If a collision still happens, e_db_pdo::copyRow() retries up to 3 times for a successful copy.

Fixes: #3678
Broken due to Codeception moving modules out of codeception/codeception
Fixes: "The method copyRow() has an NPath complexity of 252. The configured NPath complexity threshold is 200."
@Deltik Deltik force-pushed the test-harness branch 2 times, most recently from 578ef7e to 1aff15d Compare December 28, 2019 15:33
@Deltik Deltik force-pushed the test-harness branch 3 times, most recently from 1971afd to 450837f Compare December 28, 2019 17:22
- NEW: Logo at the top
- MOD: Moved Gitter badge to the end
- MOD: Added e107 logo to GitHub release badge
- MOD: Replaced GitHub Workflow badge with Shields.io equivalent for consistency
- MOD: Replaced Codecov code coverage badge with Code Climate equivalent
- MOD: Added Gitter logo and styling to Gitter badge
@codeclimate
Copy link

codeclimate bot commented Dec 28, 2019

Code Climate has analyzed commit 8d5d184 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 47.5% (80% is the threshold).

This pull request will bring the total coverage in the repository to 5.4%.

View more on Code Climate.

Copy link
Member

@CaMer0n CaMer0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!! :-)

@CaMer0n CaMer0n merged commit 598522e into master Dec 28, 2019
@Deltik Deltik deleted the test-harness branch December 29, 2019 14:58
@Deltik Deltik mentioned this pull request Dec 28, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core: testing framework type: enhancement An improvement or new feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants