Global libraries tests

[azizpunjani]

Closes #1009
Closes #1008
Closes #1007
Closes #1006
Closes #1003
Closes #1002
Closes #1004
Closes #1001
Closes #1000
Closes #999
Closes #998
Closes #997
Closes #996
Closes #995
Closes #994
Closes #993
Closes #992
Closes #991
Closes #990
Closes #989
Closes #988
Closes #734
Closes #733
Closes #732
Closes #729
Closes #728

[notmessenger]

• Test multiple rendered instances #1004 does not seem to be address in this PR
• I know that there were not issues for these, and that they are not components intended to be directly used, and that they don't do anything DOM related, but we should we also test these components the same way, precisely to make sure they don't start doing DOM things in a bad way?
  • sl-menu-item-show-all
  • sl-modal-body
  • /sl-modal-footer
  • sl-modal-header

---

Reviewed 34 of 34 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

tests/.jshintrc, line 5 [r1] (raw file):
This is a duplicate entry

---

tests/unit/components/sl-date-time-test.js, line 307 [r1] (raw file):
Remove this added empty line

---

Comments from the review on Reviewable.io

[juwara0]

There is no message included in the assert for these tests.

 assert.notOk(
        globalLibraries.called()
    );

Please add a message:

 assert.notOk(
        globalLibraries.called(),
       'Need a message here'
    );

---

Reviewed 19 of 34 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from the review on Reviewable.io

[azizpunjani]

I don't think a message is needed because that is the only assert in the test, @notmessenger can probably weight in on this?

---

Comments from the review on Reviewable.io

[juwara0]

softlayer/ember-style-guide#79

---

Comments from the review on Reviewable.io

[azizpunjani]

Created #1463 based off #1459 (comment).

[azizpunjani]

Review status: 3 of 38 files reviewed at latest revision, 2 unresolved discussions.

---

tests/.jshintrc, line 5 [r1] (raw file):
Removed duplicate. Sorted alphabetically to avoid duplicates.

---

tests/unit/components/sl-date-time-test.js, line 307 [r1] (raw file):
Done.

---

Comments from the review on Reviewable.io

[juwara0]

Reviewed 1 of 34 files at r1, 22 of 34 files at r2, 2 of 2 files at r3.
Review status: 27 of 38 files reviewed at latest revision, 2 unresolved discussions.

---

Comments from the review on Reviewable.io

[notmessenger]

• Test multiple rendered instances #1004 does not seem to be address in this PR

---

Reviewed 33 of 34 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[azizpunjani]

@notmessenger 1004 has now been addressed.

[notmessenger]

Reviewed 7 of 7 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

tests/unit/components/sl-alert-test.js, line 95 [r4] (raw file):
Haven't we, if only by convention, been placing these tests cases at/near the end of the file?

---

tests/unit/components/sl-grid-test.js, line 901 [r4] (raw file):
The 3 assert calls are necessary vs the pattern in most of the other ones?

---

Comments from the review on Reviewable.io

[azizpunjani]

Review status: 38 of 39 files reviewed at latest revision, 2 unresolved discussions.

---

tests/unit/components/sl-alert-test.js, line 95 [r4] (raw file):
Yes, moved the test to the end of the file.

---

tests/unit/components/sl-grid-test.js, line 901 [r4] (raw file):
They are necessary because the grid does reference Ember.$ but not jQuery or $.

---

Comments from the review on Reviewable.io

[notmessenger]

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io