Skip to content

PHP Unit Testing Updates#97

Merged
alektrefi merged 9 commits into
developfrom
is-bu-domain-tests-update
Nov 6, 2018
Merged

PHP Unit Testing Updates#97
alektrefi merged 9 commits into
developfrom
is-bu-domain-tests-update

Conversation

@alektrefi
Copy link
Copy Markdown
Contributor

@alektrefi alektrefi commented Oct 2, 2018

  • Update Travis version of WordPress Test to match our current version.
  • Update is_responsive_bu_domain tests to use different test for multi vs single site.

[X] I've reviewed the contribution guidelines.
[X] I've updated CHANGELOG.MD with a brief explanation of the changes in this pull request in the unreleased section.
[X] My code follows BU Coding Standards.

@@ -148,18 +148,39 @@ function test_responsive_is_bu_domain() {
* Test that BU domains correctly match.
*/
function test_responsive_is_bu_domain_true() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I always wondered if this truly the full list of subdomains we should check for (what about sites.bu.edu, sites-staging.bu.edu, ect.). Also do we need to worry about this catching domains like agganisarena.com, citycoboston.com, others?

Copy link
Copy Markdown
Contributor Author

@alektrefi alektrefi Oct 18, 2018

Choose a reason for hiding this comment

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

This is a good question. My initial goal here was to get the broken test working again. I appreciate how having the Unit tests prompted this deeper follow up.

Currently the function is only looking for bu.edu in the URL

function responsive_is_bu_domain() {
return (bool) preg_match( '#\bbu\.edu\b#', network_home_url() );
}

The functions only current use is to add a meta value with the Boston University name and Copyright. With branding guidelines do we want sites.bu.edu to get the BU Copyright key? Also, do we want agganisarena.com, citycoboston.com to get this copyright meta value? What about blogs.bu.edu?

<?php if ( responsive_is_bu_domain() ) : ?>
<meta name="copyright" content="&copy; <?php echo esc_attr( date( 'Y' ) ); ?> Boston University" />
<?php else : ?>
<link rel="shortcut icon" href="<?php bloginfo( 'template_directory' ); ?>/icons/favicon-g.ico" />
<?php endif; ?>

If we do want these sites to get the copyright meta value, then we need to update the function and the test.

Who can confirm the list of sites and subdomains that need to get the Boston University copyright meta value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As discussed in person, I opened an issue for the broader question. #110

Please approve so we can close this as is and fix the tests.

@toddmilliken
Copy link
Copy Markdown
Contributor

toddmilliken commented Oct 25, 2018

This looks good to me also!

Just for more context: I had updated the responsive_is_bu_domain function inside inc/template-tags.php to resolve a fatal error that occured on single site installs when responsive framework was added. efe3113#diff-5915276465e785d5f7897d71685fca5f resolves PR #3

* develop: (1259 commits)
  Run grunt build to complete version bump to 2.1.9
  Bump version to 2.1.9 and update CHANGELOG
  Fix CHANGELOG nested ul indendation
  Fix multiple Code Climate issues on CHANGELOG
  Update CHANGELOG with bugfix notes
  Update CHANGELOG to use level 2 headings
  Bugfix: Prevent empty post title from rendering
  Bugfix: Fix typo in name of customizer filter
  version bump
  add unit tests for r_page_title_class
  add $display parameter to function update templates to use parameter
  Add filter to filter the page title class
  removing stray junk
  updating tests
  restoring code to old working version
  Add the global favicon back for non-BU domains
  version bump
  Temporarily remove the `r_remove_news_template` filter
  Move the `include` out of the `if`
  Update README.md
  ...

# Conflicts:
#	CHANGELOG.md
@alektrefi alektrefi merged commit d7394a4 into develop Nov 6, 2018
@alektrefi alektrefi deleted the is-bu-domain-tests-update branch November 6, 2018 19:46
alektrefi added a commit that referenced this pull request Nov 6, 2018
* develop: (1254 commits)
  PHP Unit Testing Updates (#97)
  Run grunt build to complete version bump to 2.1.9
  Bump version to 2.1.9 and update CHANGELOG
  version bump
  add unit tests for r_page_title_class
  add $display parameter to function update templates to use parameter
  Add filter to filter the page title class
  removing stray junk
  updating tests
  restoring code to old working version
  Add the global favicon back for non-BU domains
  version bump
  Temporarily remove the `r_remove_news_template` filter
  Move the `include` out of the `if`
  Update README.md
  add contributor
  Version Bump 2.0.1
  Code Climate Revision 2
  Code Climate Revision
  Hub Indicator Style Integration
  ...
alektrefi added a commit that referenced this pull request Nov 7, 2018
* develop: (1255 commits)
  Migrate Open PR #418 from 1x-develop fix deprecated GF call (#107)
  PHP Unit Testing Updates (#97)
  Run grunt build to complete version bump to 2.1.9
  Bump version to 2.1.9 and update CHANGELOG
  version bump
  add unit tests for r_page_title_class
  add $display parameter to function update templates to use parameter
  Add filter to filter the page title class
  removing stray junk
  updating tests
  restoring code to old working version
  Add the global favicon back for non-BU domains
  version bump
  Temporarily remove the `r_remove_news_template` filter
  Move the `include` out of the `if`
  Update README.md
  add contributor
  Version Bump 2.0.1
  Code Climate Revision 2
  Code Climate Revision
  ...

# Conflicts:
#	languages/responsive-framework.pot
alektrefi added a commit that referenced this pull request Nov 7, 2018
* develop: (1255 commits)
  Migrate Open PR #418 from 1x-develop fix deprecated GF call (#107)
  PHP Unit Testing Updates (#97)
  Run grunt build to complete version bump to 2.1.9
  Bump version to 2.1.9 and update CHANGELOG
  version bump
  add unit tests for r_page_title_class
  add $display parameter to function update templates to use parameter
  Add filter to filter the page title class
  removing stray junk
  updating tests
  restoring code to old working version
  Add the global favicon back for non-BU domains
  version bump
  Temporarily remove the `r_remove_news_template` filter
  Move the `include` out of the `if`
  Update README.md
  add contributor
  Version Bump 2.0.1
  Code Climate Revision 2
  Code Climate Revision
  ...

# Conflicts:
#	languages/responsive-framework.pot
alektrefi added a commit that referenced this pull request Nov 9, 2018
* develop: (1262 commits)
  Migrate Open PR #407 from 1x-develop Security Audit June 2018 (#106)
  Migrate Open PR #418 from 1x-develop fix deprecated GF call (#107)
  PHP Unit Testing Updates (#97)
  Run grunt build to complete version bump to 2.1.9
  Bump version to 2.1.9 and update CHANGELOG
  Fix CHANGELOG nested ul indendation
  Fix multiple Code Climate issues on CHANGELOG
  Update CHANGELOG with bugfix notes
  Update CHANGELOG to use level 2 headings
  Bugfix: Prevent empty post title from rendering
  Bugfix: Fix typo in name of customizer filter
  version bump
  add unit tests for r_page_title_class
  add $display parameter to function update templates to use parameter
  Add filter to filter the page title class
  removing stray junk
  updating tests
  restoring code to old working version
  Add the global favicon back for non-BU domains
  version bump
  ...

# Conflicts:
#	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants