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

Karma Init #1557

Merged
merged 11 commits into from
Aug 2, 2020
Merged

Karma Init #1557

merged 11 commits into from
Aug 2, 2020

Conversation

ksorv
Copy link
Collaborator

@ksorv ksorv commented Jul 26, 2020

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
New tests added? not needed
License MIT

Description

  • Moving to Karma for handling tests.
  • Moved to BrowserStack from SauceLabs
  • Coveralls Working
  • Tests passing on Browsers
    • Win 10 Chrome
    • Win 10 Firefox
    • Win 10 Edge
    • Win 8.1 Chrome
    • Win 8.1 Edge
    • OSX El Capitan Chrome
    • OSX El Capitan Firefox
    • OSX El Capitan Safari

--

Please, don't submit /dist files with your PR!

@ksorv
Copy link
Collaborator Author

ksorv commented Jul 26, 2020

#1552

Tests passing again!
Browser-Stack being utilized.

Some tests are not passing!

@ksorv
Copy link
Collaborator Author

ksorv commented Jul 26, 2020

Firefox has errors which related to execAction(font props) of Core-API.

  • fontSize support old style
  • fontName support new stle
  • fontSize support new stle
  • fontName support old style

Not related to fonts probably!

  • should pass opt directly to document.execCommand
Firefox 78.0 (Windows 10): Executed 617 of 617 (5 FAILED) (18.184 secs / 10.708 secs)
TOTAL: 5 FAILED, 612 SUCCESS


1) fontSize support old style
     Core-API execAction
     TypeError: MediumEditor.util.getContainerEditorElement(...).focus is not a function in src/js/core.js (line 962)
execAction@src/js/core.js:962:94
@spec/core-api.spec.js:239:20
<Jasmine>

2) fontName support new stle
     Core-API execAction
     TypeError: MediumEditor.util.getContainerEditorElement(...).focus is not a function in src/js/core.js (line 962)
execAction@src/js/core.js:962:94
@spec/core-api.spec.js:225:20
<Jasmine>

3) fontSize support new stle
     Core-API execAction
     TypeError: MediumEditor.util.getContainerEditorElement(...).focus is not a function in src/js/core.js (line 962)
execAction@src/js/core.js:962:94
@spec/core-api.spec.js:253:20
<Jasmine>

4) fontName support old style
     Core-API execAction
     TypeError: MediumEditor.util.getContainerEditorElement(...).focus is not a function in src/js/core.js (line 962)
execAction@src/js/core.js:962:94
@spec/core-api.spec.js:211:20
<Jasmine>

5) should pass opt directly to document.execCommand
     Core-API execAction
     TypeError: MediumEditor.util.getContainerEditorElement(...).focus is not a function in src/js/core.js (line 962)
execAction@src/js/core.js:962:94
@spec/core-api.spec.js:197:20
<Jasmine>


TOTAL: 5 FAILED, 612 SUCCESS

@ksorv
Copy link
Collaborator Author

ksorv commented Jul 26, 2020

Safari

  • Resolved.

Well, It sucks, I don't have a Mac to debug these.

Safari 8.0.8 (Mac OS 10.10.5): Executed 619 of 617 (4 FAILED) (3.472 secs / 3.6 secs)
TOTAL: 4 FAILED, 615 SUCCESS


1) should close the font name form when user clicks on cancel
     Font Name Button TestCase Cancel
     IndexSizeError: DOM Exception 1: Index or size was negative, or greater than the allowed value. thrown

2) should close the font name form when user clicks on cancel
     Font Name Button TestCase Cancel
     IndexSizeError: DOM Exception 1: Index or size was negative, or greater than the allowed value. thrown
     TypeError: undefined is not a function (evaluating 'this.cleanupTest()') in spec/fontname.spec.js (line 26)
spec/fontname.spec.js:26:25
<Jasmine>

3) should close the font size form when user clicks on cancel
     Font Size Button TestCase Cancel
     IndexSizeError: DOM Exception 1: Index or size was negative, or greater than the allowed value. thrown

4) should close the font size form when user clicks on cancel
     Font Size Button TestCase Font Size
     IndexSizeError: DOM Exception 1: Index or size was negative, or greater than the allowed value. thrown
     TypeError: undefined is not a function (evaluating 'this.cleanupTest()') in spec/fontsize.spec.js (line 26)
spec/fontsize.spec.js:26:25
<Jasmine>


TOTAL: 4 FAILED, 615 SUCCESS

Edit: Was cause of CSS files not being included in srcFiles in Karma Config.

@ksorv ksorv linked an issue Jul 26, 2020 that may be closed by this pull request
9 tasks
Copy link
Collaborator Author

@ksorv ksorv left a comment

Choose a reason for hiding this comment

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

Might be better if we create a util, isEmpty(obj) to check that. Or this is just fine, I guess.

src/js/core.js Show resolved Hide resolved
@ksorv ksorv removed a link to an issue Jul 27, 2020
9 tasks
@ksorv
Copy link
Collaborator Author

ksorv commented Jul 27, 2020

IE is used by <2% users in the world. I think we should just drop it.
@roniemartinez @arteconceito
What you think about IE support!

@arteconceito
Copy link
Contributor

IE is used by <2% users in the world. I think we should just drop it.
@roniemartinez What you think about IE support!

Browser Share

In general public usage yes but believe me, there are still many companies (I mean at the enterpise level) that still and only use IE 11.
I suppose this should be more a question of... Do we want to support it or not?

By the way, nice work with the tests migration. Unfortunately, I could not yet find the time I wished to contribute to the project!

@ksorv
Copy link
Collaborator Author

ksorv commented Jul 27, 2020

Yeah, I think the same too.

I think we should go for v10 and v11 as most people used to use ME cause it used to support all browsers.

Yeah, I got some free time(more like spirit than time) to do this. As i barely write tests.

And I'm still clueless on debugging these tests on IE and supporting them.

@arteconceito Direct me? I'll try to do it.

@arteconceito
Copy link
Contributor

@sauravkhdoolia in my opinion supporting IE v11 is good enough and should cover most of IE user base as of today.

In order to debug IE I usually use a VM running IE11 on it. Firebug is also a good debugging extension as IE dev tools are not the best.

@ksorv
Copy link
Collaborator Author

ksorv commented Jul 27, 2020

I'll start there. Thanks brother.

@ksorv
Copy link
Collaborator Author

ksorv commented Jul 30, 2020

Hey @arteconceito It works on IE11 sometimes and sometimes it doesn't.

That is so weird to catch, Why sometimes an API is available in the IE browser, and sometimes it's not.

Here:

@ksorv
Copy link
Collaborator Author

ksorv commented Jul 30, 2020

@arteconceito It works in IE11, but sometimes it doesn't.

I don't know why?

Here:

Medium Edittor IE test screenshot

@ksorv ksorv added the tests label Jul 30, 2020
@ksorv
Copy link
Collaborator Author

ksorv commented Jul 30, 2020

IE Errors found on BrowserStack.

IE 11.0 (Windows 10): Executed 617 of 617 (2 FAILED) (20.473 secs / 12.935 secs)
TOTAL: 2 FAILED, 615 SUCCESS


1) should change font name when select is changed
     Font Name Button TestCase Font Name
     Error: <toHaveBeenCalledWith> : Expected a spy, but got Function.
Usage: expect(<spyObj>).toHaveBeenCalledWith(...arguments)
Error: <toHaveBeenCalledWith> : Expected a spy, but got Function.
Usage: expect(<spyObj>).toHaveBeenCalledWith(...arguments)
    at <Jasmine>
   at Anonymous function (spec/fontname.spec.js:70:13)
    at <Jasmine>

2) should revert font name when select value is set to empty
     Font Name Button TestCase Font Name
     Error: <toHaveBeenCalledWith> : Expected a spy, but got Function.
Usage: expect(<spyObj>).toHaveBeenCalledWith(...arguments)
Error: <toHaveBeenCalledWith> : Expected a spy, but got Function.
Usage: expect(<spyObj>).toHaveBeenCalledWith(...arguments)
    at <Jasmine>
   at Anonymous function (spec/fontname.spec.js:104:13)
    at <Jasmine>


TOTAL: 2 FAILED, 615 SUCCESS

@ksorv ksorv merged commit 2c0af6b into master Aug 2, 2020
@j0k3r j0k3r deleted the karma branch October 24, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants