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

Issue/12342/js test driver removal #12406

Merged
merged 36 commits into from
Jan 20, 2018
Merged

Issue/12342/js test driver removal #12406

merged 36 commits into from
Jan 20, 2018

Conversation

KarlDeux
Copy link

@KarlDeux KarlDeux commented Nov 22, 2017

Fixes #12342

Refactoring and creation of Jasmine Unit testing.
Removed JSTestDriver.

Description

Currently Magento is using 2 different sets of tools for the JavaScript Testing:

JSTestDriver, which is considered legacy and is not supported anymore by the core team.
Jasmine, which was introduced to replace JSTestDriver. All new JS tests are implemented using it and executed in multiple CI environments, including public Travis CI and Magento's in-house CICD infrastructure.
To remove legacy framework it is required to reimplement remaining tests using Jasmine, and completely remove JSTestDriver support afterwards.

Fixed Issues (if relevant)

  1. JSTestDriver removal #12342 : JSTestDriver removal

Manual testing scenarios

  1. Make sure that NPM is installed, if not $ npm install.
  2. Generate static view files in Magento that are going to be tested $ php bin/magento setup:static-content:deploy -f.
  3. Run Jasmine testing $ grunt spec:.

All the tests should pass.

List of JS which require attention:

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@KarlDeux
Copy link
Author

KarlDeux commented Nov 22, 2017

@ishakhsuvarov @magento-engcom-team are we still using lib/web/mage/translate-inline.js
JsTestDriver/testsuite/mage/translate_inline/translate-inline-test.js ?

Couldn't find any usage into the magento 2 and those tests are hard to deal with since that'd involve DOM inclusions. Without jasmine-jquery it'd be hard.

Carlos Lizaga added 2 commits November 22, 2017 17:50
Removed accordion.js and index.html for jsTestDriver test.
@omiroshnichenko
Copy link
Contributor

Hi, @KarlDeux. Thank you for your interest in this task. We still using translate-inline in admin and front areas. For such case, you can create separate html file and fetch it using require text plugin.

@KarlDeux
Copy link
Author

Ok @omiroshnichenko will do so.

@omiroshnichenko
Copy link
Contributor

@KarlDeux Im have already rewritten decorate-test.js. Is it be ok if I will make commit to your branch?

@KarlDeux
Copy link
Author

KarlDeux commented Nov 22, 2017

@omiroshnichenko sure thing! I'm fixing now accordion and finishing collapsible.

@KarlDeux
Copy link
Author

@omiroshnichenko looks like decorate test are failing:

   Error: Method customMethod does not exist on jQuery.decorate in http://localhost:8000/pub/static/frontend/Magento/blank/en_US/jquery.js (line 253) (1)

@omiroshnichenko
Copy link
Contributor

@KarlDeux My fault. Fixed.

Carlos Lizaga added 3 commits November 23, 2017 17:19
Fixed enable method. It was not enabling and activating the collapsible.
This was detected on Jasmine unit testing.

Minor repositioning to this.options.disabled on disable method for
consistency purposes with enable method.
@KarlDeux KarlDeux closed this Nov 23, 2017
@KarlDeux KarlDeux reopened this Nov 23, 2017
@KarlDeux
Copy link
Author

Had to apply some changes to lib/web/mage/collapsible.js in order to be testable.
Also discovered that enable function wasn't working properly running the tests. Did some changes so it is actually enabling the chollapsible item (enabling tab-index, showing the element), dunno if this is the actual behavior that should be applied but reading the previous test cases it'd make sense.

- Added translate-inline.test.js and removed JsTestDriver equivalent.
- Added calendar.test.js and removed JsTestDriver equivalent.
- Fix translate-inline.test.js, save original ajax
- Fix loader.test.js and removed JsTestDriver equivalent.
- Added suggest.test.js and removed JsTestDriver equivalent.
- Added tree-suggest.test.js and removed JsTestDriver equivalent.
- Added tabs.test.js and removed JsTestDriver equivalent.
@omiroshnichenko
Copy link
Contributor

@KarlDeux Maybe I can help you with validation test?

@omiroshnichenko
Copy link
Contributor

@KarlDeux If you busy, I can start work on validation-test?

@KarlDeux
Copy link
Author

@omiroshnichenko hey! I was out for vacations, just commited the file as I have it (still needs work). Feel free to finish it :)

@omiroshnichenko
Copy link
Contributor

@KarlDeux Ohhh!!! I just finished all test and ready to commit. Maybe I will make force push if you do not mind?

@KarlDeux
Copy link
Author

Sure @omiroshnichenko ! go for it!

- Added validation.test.js and removed JsTestDriver equivalent.
- Fix unstable tests.
- Fix timeout dropdown close.
- Fix jstree override.
@magento-engcom-team
Copy link
Contributor

@KarlDeux thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@KarlDeux
Copy link
Author

@omiroshnichenko guess we're done right? Shall I squash it all or do you want to do so?

@ishakhsuvarov
Copy link
Contributor

@KarlDeux Going to merge it as is, don't see any problem in keeping history for this amount of work done 👍
Thank you!

@ishakhsuvarov ishakhsuvarov mentioned this pull request Jan 20, 2018
13 tasks
@magento-team magento-team merged commit 64f9c6d into magento:2.2-develop Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants