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

Change functional test parameters, update to jQuery3 #331

Merged

Conversation

mistergone
Copy link
Member

This code will update the functional tests to use a school from the default data (collegedata.json) rather than the test data (test_program.json) to avoid extra steps in making those functional tests work.

It also updates jQuery in the application from 1.11.3 to 3.3.1.

Changes

  • Upgrade jquery to version 3+, fixing old vulnerabilities
  • Change functional tests to use data from the 'collegedata' fixture, which is loaded automatically in standalone_setup.sh, rather than the 'test_program' fixture, which must be loaded manually

Testing

  • Run functional tests as outlined in the README.

Review

Notes

  • Some functional tests are going to be changed in a future PR

Todos

  • One pending spec and a few other specs which test the opening of external sites will be changed to test only that links are properly inserted into the text.

Checklist

  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the front end playbook
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged
  • Visually tested in supported browsers and devices
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

@coveralls
Copy link

coveralls commented Sep 7, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling c391925 on no-really-the-functional-tests-are-fixed-now-seriously into 8826655 on master.

@mistergone mistergone force-pushed the no-really-the-functional-tests-are-fixed-now-seriously branch from c66739e to e742cd9 Compare September 7, 2018 14:49
- Change functional tests to use data from the 'collegedata' fixture,
which is loaded automatically in standalone_setup.sh, rather than
the 'test_program' fixture, which must be loaded manually
- Upgrade jquery to version 3+, fixing old vulnerabilities
- Clean up some commented code
@mistergone mistergone force-pushed the no-really-the-functional-tests-are-fixed-now-seriously branch from e742cd9 to 0e5dc04 Compare September 7, 2018 15:46
@@ -3,8 +3,8 @@ exports.config = {
seleniumAddress: 'http://localhost:4444/wd/hub',
capabilities: { 'browserName': 'chrome' },
specs: ['dd-functional-settlement-spec.js', 'dd-settlement-only-spec.js', 'dd-feedback-spec.js', 'dd-school-data-spec.js', 'dd-interactions-spec.js' ],
// specs: ['dd-interactions-spec.js'],
// specs: ['dd-functional-settlement-spec.js'],
// By limiting the specs to one file, we can isolate and fix specific tests:
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -130,7 +130,7 @@ fdescribe( 'The dynamic financial aid disclosure', function() {
expect( page.defaultRateLink.getAttribute( 'href' ) ).toEqual( 'http://nces.ed.gov/collegenavigator/?id=182111#fedloans' );
} );

it( 'should open the link to the College Navigator cohort loan default rates in a new tab with the loan default rates section open', function() {
fit( 'should open the link to the College Navigator cohort loan default rates in a new tab with the loan default rates section open', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

intentional? If so, let's add a comment to explain.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unintentional! Thanks for the catch! New commit fixes this.

@@ -722,7 +720,7 @@ it( 'should properly update when more than one private loans is modified', funct
} );

it( 'should properly describe a future based on covering more of the cost of college that is needed', function() {
browser.get( 'http://localhost:8000/paying-for-college2/understanding-your-financial-aid-offer/offer/?iped=408039&pid=981&oid=9e0280139f3238cbc9702c7b0d62e5c238a835a0&book=650&gib=3000&gpl=1000&hous=3000&insi=4.55&insl=3000&inst=36&mta=3000&othg=100&othr=500&parl=10000&pelg=1500&perl=3000&ppl=1000&prvl=3000&prvf=2.1&prvi=4.55&schg=2000&stag=2000&subl=3500&tran=500&tuit=38976&unsl=2000&wkst=3000' );
browser.get( 'http://localhost:8000/paying-for-college2/understanding-your-financial-aid-offer/offer/?iped=182111&pid=1412&oid=9e0280139f3238cbc9702c7b0d62e5c238a835a0&book=650&gib=3000&gpl=1000&hous=3000&insi=4.55&insl=3000&inst=36&mta=3000&othg=100&othr=500&parl=10000&pelg=1500&perl=3000&ppl=1000&prvl=3000&prvf=2.1&prvi=4.55&schg=2000&stag=2000&subl=3500&tran=500&tuit=38976&unsl=2000&wkst=3000' );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line got de-dented. I assume that's just a typo from commenting/uncommenting?

@schbetsy
Copy link
Collaborator

schbetsy commented Sep 7, 2018

This is passing for me, when I remove the fit*! And it looks to be more stable than the previous version (I've done a few trial runs and haven't seen any intermittent failures so far).

(* There's still one failure, where it tries to open an 3rd party site and times out. As we discussed, we're adding a backlog task to keep the functional tests from doing that, so I'm not worried about it here.)

- Change fit() to it() in dd-settlement-only-spec.js
- Fix indentation in dd-functional-settlement-spec.js
@mistergone mistergone force-pushed the no-really-the-functional-tests-are-fixed-now-seriously branch from 7ce4efa to c391925 Compare September 7, 2018 17:12
Copy link
Collaborator

@schbetsy schbetsy left a comment

Choose a reason for hiding this comment

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

Looks good!

@mistergone mistergone merged commit 6772512 into master Sep 7, 2018
@schbetsy schbetsy deleted the no-really-the-functional-tests-are-fixed-now-seriously branch September 7, 2018 17:57
@chosak chosak mentioned this pull request Sep 11, 2018
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.

None yet

3 participants