From 6f25f1736031184ac05fc4059edde772a7e3bf56 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 2 Feb 2022 12:09:03 -0500 Subject: [PATCH] DEV: Revisit skipped tests (#15769) * Some are no longer flaky or easily fixed * Some are out of date or test things we can't do accurately (scroll position) and are removed. * Unwinds some uppy tests and makes sure all promises and runloops are resolved. Everything has been run in legacy/ember cli multiple times to ensure no obvious suite regressions. --- .../app/mixins/composer-upload-uppy.js | 218 ++++++++++-------- .../tests/acceptance/click-track-test.js | 5 +- .../acceptance/composer-uploads-uppy-test.js | 45 ++-- .../tests/acceptance/fast-edit-test.js | 4 +- .../tests/acceptance/jump-to-test.js | 4 +- .../discourse/tests/acceptance/modal-test.js | 6 +- .../tests/acceptance/post-history-test.js | 4 +- .../tests/acceptance/search-full-test.js | 44 +--- .../tests/acceptance/sign-in-test.js | 8 +- .../tests/acceptance/sticky-avatars-test.js | 19 -- .../acceptance/topic-quote-button-test.js | 155 +++++++------ .../tests/acceptance/unknown-test.js | 4 +- .../discourse/tests/helpers/qunit-helpers.js | 12 +- .../integration/components/ace-editor-test.js | 24 +- .../integration/components/value-list-test.js | 4 - .../integration/widgets/home-logo-test.js | 1 - .../unit/lib/click-track-edit-history-test.js | 107 --------- .../unit/lib/click-track-profile-page-test.js | 98 -------- .../tests/unit/lib/load-script-test.js | 19 +- .../tests/unit/lib/pretty-text-test.js | 16 +- .../tests/unit/lib/utilities-test.js | 19 +- 21 files changed, 261 insertions(+), 555 deletions(-) delete mode 100644 app/assets/javascripts/discourse/tests/acceptance/sticky-avatars-test.js delete mode 100644 app/assets/javascripts/discourse/tests/unit/lib/click-track-edit-history-test.js delete mode 100644 app/assets/javascripts/discourse/tests/unit/lib/click-track-profile-page-test.js diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js index 7ba98ad19ef..5039c8626bd 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js @@ -20,6 +20,7 @@ import { } from "discourse/lib/uploads"; import { cacheShortUploadUrl } from "pretty-text/upload-short-url"; import bootbox from "bootbox"; +import { run } from "@ember/runloop"; // Note: This mixin is used _in addition_ to the ComposerUpload mixin // on the composer-editor component. It overrides some, but not all, @@ -206,112 +207,135 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, { } this._uppyInstance.on("file-added", (file) => { - if (isPrivateMessage) { - file.meta.for_private_message = true; - } + run(() => { + if (isPrivateMessage) { + file.meta.for_private_message = true; + } + }); }); this._uppyInstance.on("progress", (progress) => { - if (this.isDestroying || this.isDestroyed) { - return; - } + run(() => { + if (this.isDestroying || this.isDestroyed) { + return; + } - this.set("uploadProgress", progress); + this.set("uploadProgress", progress); + }); }); this._uppyInstance.on("file-removed", (file, reason) => { - // we handle the cancel-all event specifically, so no need - // to do anything here. this event is also fired when some files - // are handled by an upload handler - if (reason === "cancel-all") { - return; - } + run(() => { + // we handle the cancel-all event specifically, so no need + // to do anything here. this event is also fired when some files + // are handled by an upload handler + if (reason === "cancel-all") { + return; + } - file.meta.cancelled = true; - this._removeInProgressUpload(file.id); - this._resetUpload(file, { removePlaceholder: true }); - if (this.inProgressUploads.length === 0) { - this.set("userCancelled", true); - this._uppyInstance.cancelAll(); - } + file.meta.cancelled = true; + this._removeInProgressUpload(file.id); + this._resetUpload(file, { removePlaceholder: true }); + if (this.inProgressUploads.length === 0) { + this.set("userCancelled", true); + this._uppyInstance.cancelAll(); + } + }); }); this._uppyInstance.on("upload-progress", (file, progress) => { - if (this.isDestroying || this.isDestroyed) { - return; - } + run(() => { + if (this.isDestroying || this.isDestroyed) { + return; + } - const upload = this.inProgressUploads.find((upl) => upl.id === file.id); - if (upload) { - const percentage = Math.round( - (progress.bytesUploaded / progress.bytesTotal) * 100 - ); - upload.set("progress", percentage); - } + const upload = this.inProgressUploads.find((upl) => upl.id === file.id); + if (upload) { + const percentage = Math.round( + (progress.bytesUploaded / progress.bytesTotal) * 100 + ); + upload.set("progress", percentage); + } + }); }); this._uppyInstance.on("upload", (data) => { - this._addNeedProcessing(data.fileIDs.length); + run(() => { + this._addNeedProcessing(data.fileIDs.length); - const files = data.fileIDs.map((fileId) => - this._uppyInstance.getFile(fileId) - ); - - this.setProperties({ - isProcessingUpload: true, - isCancellable: false, - }); - - files.forEach((file) => { - // The inProgressUploads is meant to be used to display these uploads - // in a UI, and Ember will only update the array in the UI if pushObject - // is used to notify it. - this.inProgressUploads.pushObject( - EmberObject.create({ - fileName: file.name, - id: file.id, - progress: 0, - extension: file.extension, - }) + const files = data.fileIDs.map((fileId) => + this._uppyInstance.getFile(fileId) ); - const placeholder = this._uploadPlaceholder(file); - this.placeholders[file.id] = { - uploadPlaceholder: placeholder, - }; - this.appEvents.trigger(`${this.eventPrefix}:insert-text`, placeholder); - this.appEvents.trigger(`${this.eventPrefix}:upload-started`, file.name); + + this.setProperties({ + isProcessingUpload: true, + isCancellable: false, + }); + + files.forEach((file) => { + // The inProgressUploads is meant to be used to display these uploads + // in a UI, and Ember will only update the array in the UI if pushObject + // is used to notify it. + this.inProgressUploads.pushObject( + EmberObject.create({ + fileName: file.name, + id: file.id, + progress: 0, + extension: file.extension, + }) + ); + const placeholder = this._uploadPlaceholder(file); + this.placeholders[file.id] = { + uploadPlaceholder: placeholder, + }; + this.appEvents.trigger( + `${this.eventPrefix}:insert-text`, + placeholder + ); + this.appEvents.trigger( + `${this.eventPrefix}:upload-started`, + file.name + ); + }); }); }); this._uppyInstance.on("upload-success", (file, response) => { - this._removeInProgressUpload(file.id); - let upload = response.body; - const markdown = this.uploadMarkdownResolvers.reduce( - (md, resolver) => resolver(upload) || md, - getUploadMarkdown(upload) - ); + run(() => { + if (!this._uppyInstance) { + return; + } + this._removeInProgressUpload(file.id); + let upload = response.body; + const markdown = this.uploadMarkdownResolvers.reduce( + (md, resolver) => resolver(upload) || md, + getUploadMarkdown(upload) + ); - cacheShortUploadUrl(upload.short_url, upload); + cacheShortUploadUrl(upload.short_url, upload); - this.appEvents.trigger( - `${this.eventPrefix}:replace-text`, - this.placeholders[file.id].uploadPlaceholder.trim(), - markdown - ); + this.appEvents.trigger( + `${this.eventPrefix}:replace-text`, + this.placeholders[file.id].uploadPlaceholder.trim(), + markdown + ); - this._resetUpload(file, { removePlaceholder: false }); - this.appEvents.trigger( - `${this.eventPrefix}:upload-success`, - file.name, - upload - ); + this._resetUpload(file, { removePlaceholder: false }); + this.appEvents.trigger( + `${this.eventPrefix}:upload-success`, + file.name, + upload + ); + }); }); this._uppyInstance.on("upload-error", this._handleUploadError); this._uppyInstance.on("complete", () => { - this.appEvents.trigger(`${this.eventPrefix}:all-uploads-complete`); - this._reset(); + run(() => { + this.appEvents.trigger(`${this.eventPrefix}:all-uploads-complete`); + this._reset(); + }); }); this._uppyInstance.on("cancel-all", () => { @@ -319,11 +343,13 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, { // only do the manual cancelling work if the user clicked cancel if (this.userCancelled) { Object.values(this.placeholders).forEach((data) => { - this.appEvents.trigger( - `${this.eventPrefix}:replace-text`, - data.uploadPlaceholder, - "" - ); + run(() => { + this.appEvents.trigger( + `${this.eventPrefix}:replace-text`, + data.uploadPlaceholder, + "" + ); + }); }); this.set("userCancelled", false); @@ -415,21 +441,25 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, { this._onPreProcessComplete( (file) => { - let placeholderData = this.placeholders[file.id]; - this.appEvents.trigger( - `${this.eventPrefix}:replace-text`, - placeholderData.processingPlaceholder, - placeholderData.uploadPlaceholder - ); + run(() => { + let placeholderData = this.placeholders[file.id]; + this.appEvents.trigger( + `${this.eventPrefix}:replace-text`, + placeholderData.processingPlaceholder, + placeholderData.uploadPlaceholder + ); + }); }, () => { - this.setProperties({ - isProcessingUpload: false, - isCancellable: true, + run(() => { + this.setProperties({ + isProcessingUpload: false, + isCancellable: true, + }); + this.appEvents.trigger( + `${this.eventPrefix}:uploads-preprocessing-complete` + ); }); - this.appEvents.trigger( - `${this.eventPrefix}:uploads-preprocessing-complete` - ); } ); }, diff --git a/app/assets/javascripts/discourse/tests/acceptance/click-track-test.js b/app/assets/javascripts/discourse/tests/acceptance/click-track-test.js index 00e1f45ccbc..3fde9f90b2c 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/click-track-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/click-track-test.js @@ -4,8 +4,7 @@ import { exists, } from "discourse/tests/helpers/qunit-helpers"; import { click, currentURL, visit } from "@ember/test-helpers"; -import { skip } from "qunit"; -// import { test } from "qunit"; +import { test } from "qunit"; acceptance("Click Track", function (needs) { let tracked = false; @@ -16,7 +15,7 @@ acceptance("Click Track", function (needs) { }); }); - skip("Do not track mentions", async function (assert) { + test("Do not track mentions", async function (assert) { await visit("/t/internationalization-localization/280"); assert.ok(!exists(".user-card.show"), "card should not appear"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js index 5ee335fe016..efb6bb6fe6f 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js @@ -9,7 +9,8 @@ import bootbox from "bootbox"; import { authorizedExtensions } from "discourse/lib/uploads"; import { click, fillIn, visit } from "@ember/test-helpers"; import I18n from "I18n"; -import { skip, test } from "qunit"; +import { test } from "qunit"; +import { Promise } from "rsvp"; function pretender(server, helper) { server.post("/uploads/lookup-urls", () => { @@ -132,28 +133,17 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) { appEvents.trigger("composer:add-files", [jsonFile]); }); - // TODO: Had to comment this out for now; it works fine in Ember CLI but lagging - // UI updates sink it for the old Ember for some reason. Will re-enable - // when we make Ember CLI the primary. - - skip("cancelling uploads clears the placeholders out", async function (assert) { + test("cancelling uploads clears the placeholders out", async function (assert) { await visit("/"); await click("#create-topic"); await fillIn(".d-editor-input", "The image:\n"); + + const image = createFile("avatar.png"); + const image2 = createFile("avatar2.png"); + const appEvents = loggedInUser().appEvents; - const done = assert.async(); - - appEvents.on("composer:uploads-cancelled", () => { - assert.strictEqual( - query(".d-editor-input").value, - "The image:\n", - "it should clear the cancelled placeholders" - ); - done(); - }); - let uploadStarted = 0; - appEvents.on("composer:upload-started", async () => { + appEvents.on("composer:upload-started", () => { uploadStarted++; if (uploadStarted === 2) { @@ -164,14 +154,21 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) { ); } }); - - appEvents.on("composer:uploads-preprocessing-complete", async () => { - await click("#cancel-file-upload"); + appEvents.on("composer:uploads-cancelled", () => { + assert.strictEqual( + query(".d-editor-input").value, + "The image:\n", + "it should clear the cancelled placeholders" + ); }); - const image = createFile("avatar.png"); - const image2 = createFile("avatar2.png"); - appEvents.trigger("composer:add-files", [image, image2]); + await new Promise(function (resolve) { + appEvents.on("composer:uploads-preprocessing-complete", function () { + resolve(); + }); + appEvents.trigger("composer:add-files", [image, image2]); + }); + await click("#cancel-file-upload"); }); test("should insert a newline before and after an image when pasting in the end of the line", async function (assert) { diff --git a/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js b/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js index ea91320e451..0485dbd73b5 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js @@ -5,7 +5,7 @@ import { selectText, } from "discourse/tests/helpers/qunit-helpers"; import { click, fillIn, triggerKeyEvent, visit } from "@ember/test-helpers"; -import { skip, test } from "qunit"; +import { test } from "qunit"; import postFixtures from "discourse/tests/fixtures/post"; import { cloneJSON } from "discourse-common/lib/object"; @@ -62,7 +62,7 @@ acceptance("Fast Edit", function (needs) { assert.notOk(exists("#fast-edit-input"), "fast editor is closed"); }); - skip("Opens full composer for multi-line selection", async function (assert) { + test("Opens full composer for multi-line selection", async function (assert) { await visit("/t/internationalization-localization/280"); const textNode = query("#post_2 .cooked"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/jump-to-test.js b/app/assets/javascripts/discourse/tests/acceptance/jump-to-test.js index 9b539b7debb..94d60b9ab85 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/jump-to-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/jump-to-test.js @@ -1,6 +1,6 @@ import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers"; import { click, currentURL, fillIn, visit } from "@ember/test-helpers"; -import { skip, test } from "qunit"; +import { test } from "qunit"; acceptance("Jump to", function (needs) { needs.user(); @@ -37,7 +37,7 @@ acceptance("Jump to", function (needs) { ); }); - skip("invalid date", async function (assert) { + test("invalid date", async function (assert) { await visit("/t/internationalization-localization/280"); await click("nav#topic-progress .nums"); await click("button.jump-to-post"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/modal-test.js b/app/assets/javascripts/discourse/tests/acceptance/modal-test.js index 08b193e9537..6a33ab945be 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/modal-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/modal-test.js @@ -6,7 +6,7 @@ import { queryAll, } from "discourse/tests/helpers/qunit-helpers"; import { click, triggerKeyEvent, visit } from "@ember/test-helpers"; -import { skip, test } from "qunit"; +import { test } from "qunit"; import I18n from "I18n"; import hbs from "htmlbars-inline-precompile"; import { run } from "@ember/runloop"; @@ -30,7 +30,7 @@ acceptance("Modal", function (needs) { I18n.translations = _translations; }); - skip("modal", async function (assert) { + test("modal", async function (assert) { await visit("/"); assert.ok(!exists(".d-modal:visible"), "there is no modal at first"); @@ -51,7 +51,7 @@ acceptance("Modal", function (needs) { await click(".login-button"); assert.strictEqual(count(".d-modal:visible"), 1, "modal should reappear"); - await triggerKeyEvent("#main-outlet", "keyup", 27); + await triggerKeyEvent("#main-outlet", "keydown", 27); assert.ok(!exists(".d-modal:visible"), "ESC should close the modal"); Ember.TEMPLATES[ diff --git a/app/assets/javascripts/discourse/tests/acceptance/post-history-test.js b/app/assets/javascripts/discourse/tests/acceptance/post-history-test.js index 2a715a7f0c2..6167e0ff3a8 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/post-history-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/post-history-test.js @@ -4,7 +4,7 @@ import { count, query, } from "discourse/tests/helpers/qunit-helpers"; -import { skip } from "qunit"; +import { test } from "qunit"; acceptance("Post - History", function (needs) { needs.user(); @@ -51,7 +51,7 @@ acceptance("Post - History", function (needs) { }); }); - skip("Shows highlighted tag changes", async function (assert) { + test("Shows highlighted tag changes", async function (assert) { await visit("/t/internationalization-localization/280"); await click("article[data-post-id='419'] .edits button"); assert.equal(count(".discourse-tag"), 4); diff --git a/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js b/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js index 19169cfd2a2..97bf8312710 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js @@ -5,10 +5,9 @@ import { queryAll, selectDate, visible, - waitFor, } from "discourse/tests/helpers/qunit-helpers"; -import { click, fillIn, triggerKeyEvent, visit } from "@ember/test-helpers"; -import { skip, test } from "qunit"; +import { click, fillIn, visit } from "@ember/test-helpers"; +import { test } from "qunit"; import { SEARCH_TYPE_CATS_TAGS, SEARCH_TYPE_DEFAULT, @@ -150,45 +149,6 @@ acceptance("Search - Full Page", function (needs) { ); }); - skip("update username through advanced search ui", async function (assert) { - await visit("/search"); - await fillIn(".search-query", "none"); - await fillIn(".search-advanced-options .user-selector", "admin"); - await click(".search-advanced-options .user-selector"); - await triggerKeyEvent( - ".search-advanced-options .user-selector", - "keydown", - 8 - ); - - waitFor(assert, async () => { - assert.ok( - visible(".search-advanced-options .autocomplete"), - '"autocomplete" popup is visible' - ); - assert.ok( - exists( - '.search-advanced-options .autocomplete ul li a span.username:contains("admin")' - ), - '"autocomplete" popup has an entry for "admin"' - ); - - await click( - ".search-advanced-options .autocomplete ul li a:nth-of-type(1)" - ); - - assert.ok( - exists('.search-advanced-options span:contains("admin")'), - 'has "admin" pre-populated' - ); - assert.strictEqual( - queryAll(".search-query").val(), - "none @admin", - 'has updated search term to "none user:admin"' - ); - }); - }); - test("update category through advanced search ui", async function (assert) { const categoryChooser = selectKit( ".search-advanced-options .category-chooser" diff --git a/app/assets/javascripts/discourse/tests/acceptance/sign-in-test.js b/app/assets/javascripts/discourse/tests/acceptance/sign-in-test.js index 182abe9802b..91dcfebe14f 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sign-in-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sign-in-test.js @@ -5,7 +5,7 @@ import { queryAll, } from "discourse/tests/helpers/qunit-helpers"; import { click, fillIn, visit } from "@ember/test-helpers"; -import { skip, test } from "qunit"; +import { test } from "qunit"; acceptance("Signing In", function () { test("sign in", async function (assert) { @@ -81,7 +81,7 @@ acceptance("Signing In", function () { ); }); - skip("second factor", async function (assert) { + test("second factor", async function (assert) { await visit("/"); await click("header .login-button"); @@ -91,7 +91,6 @@ acceptance("Signing In", function () { await fillIn("#login-account-password", "need-second-factor"); await click(".modal-footer .btn-primary"); - assert.not(exists("#modal-alert:visible"), "it hides the login error"); assert.not( exists("#credentials:visible"), "it hides the username and password prompt" @@ -114,7 +113,7 @@ acceptance("Signing In", function () { ); }); - skip("security key", async function (assert) { + test("security key", async function (assert) { await visit("/"); await click("header .login-button"); @@ -124,7 +123,6 @@ acceptance("Signing In", function () { await fillIn("#login-account-password", "need-security-key"); await click(".modal-footer .btn-primary"); - assert.not(exists("#modal-alert:visible"), "it hides the login error"); assert.not( exists("#credentials:visible"), "it hides the username and password prompt" diff --git a/app/assets/javascripts/discourse/tests/acceptance/sticky-avatars-test.js b/app/assets/javascripts/discourse/tests/acceptance/sticky-avatars-test.js deleted file mode 100644 index 042f8232f55..00000000000 --- a/app/assets/javascripts/discourse/tests/acceptance/sticky-avatars-test.js +++ /dev/null @@ -1,19 +0,0 @@ -import { acceptance, query } from "discourse/tests/helpers/qunit-helpers"; -import { skip } from "qunit"; -import { visit } from "@ember/test-helpers"; - -acceptance("Sticky Avatars", function () { - skip("Adds sticky avatars when scrolling up", async function (assert) { - const container = document.getElementById("ember-testing-container"); - container.scrollTo(0, 0); - - await visit("/t/internationalization-localization/280"); - container.scrollTo(0, 800); - container.scrollTo(0, 700); - - assert.ok( - query("#post_5").parentElement.classList.contains("sticky-avatar"), - "Sticky avatar is applied" - ); - }); -}); diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js index d539c693739..6c9c7bb9c25 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js @@ -1,12 +1,13 @@ import { acceptance, + chromeTest, exists, queryAll, selectText, } from "discourse/tests/helpers/qunit-helpers"; import I18n from "I18n"; import { click, triggerKeyEvent, visit } from "@ember/test-helpers"; -import { skip, test } from "qunit"; +import { test } from "qunit"; // This tests are flaky on Firefox. Fails with `calling set on destroyed object` acceptance("Topic - Quote button - logged in", function (needs) { @@ -16,42 +17,50 @@ acceptance("Topic - Quote button - logged in", function (needs) { share_quote_buttons: "twitter|email", }); - // All these skips were chromeTest - skip("Does not show the quote share buttons by default", async function (assert) { - await visit("/t/internationalization-localization/280"); - await selectText("#post_5 blockquote"); - assert.ok(exists(".insert-quote"), "it shows the quote button"); - assert.ok(!exists(".quote-sharing"), "it does not show quote sharing"); - }); + chromeTest( + "Does not show the quote share buttons by default", + async function (assert) { + await visit("/t/internationalization-localization/280"); + await selectText("#post_5 blockquote"); + assert.ok(exists(".insert-quote"), "it shows the quote button"); + assert.ok(!exists(".quote-sharing"), "it does not show quote sharing"); + } + ); - skip("Shows quote share buttons with the right site settings", async function (assert) { - this.siteSettings.share_quote_visibility = "all"; + chromeTest( + "Shows quote share buttons with the right site settings", + async function (assert) { + this.siteSettings.share_quote_visibility = "all"; - await visit("/t/internationalization-localization/280"); - await selectText("#post_5 blockquote"); + await visit("/t/internationalization-localization/280"); + await selectText("#post_5 blockquote"); - assert.ok(exists(".quote-sharing"), "it shows the quote sharing options"); - assert.ok( - exists(`.quote-sharing .btn[title='${I18n.t("share.twitter")}']`), - "it includes the twitter share button" - ); - assert.ok( - exists(`.quote-sharing .btn[title='${I18n.t("share.email")}']`), - "it includes the email share button" - ); - }); + assert.ok(exists(".quote-sharing"), "it shows the quote sharing options"); + assert.ok( + exists(`.quote-sharing .btn[title='${I18n.t("share.twitter")}']`), + "it includes the twitter share button" + ); + assert.ok( + exists(`.quote-sharing .btn[title='${I18n.t("share.email")}']`), + "it includes the email share button" + ); + } + ); - skip("Quoting a Onebox should not copy the formatting of the rendered Onebox", async function (assert) { - await visit("/t/topic-for-group-moderators/2480"); - await selectText("#post_3 aside.onebox p"); - await click(".insert-quote"); + chromeTest( + "Quoting a Onebox should not copy the formatting of the rendered Onebox", + async function (assert) { + await visit("/t/topic-for-group-moderators/2480"); + await selectText("#post_3 aside.onebox p"); + await click(".insert-quote"); - assert.strictEqual( - queryAll(".d-editor-input").val().trim(), - '[quote="group_moderator, post:3, topic:2480"]\nhttps://example.com/57350945\n[/quote]', - "quote only contains a link" - ); - }); + assert.strictEqual( + queryAll(".d-editor-input").val().trim(), + '[quote="group_moderator, post:3, topic:2480"]\nhttps://example.com/57350945\n[/quote]', + "quote only contains a link" + ); + } + ); }); acceptance("Topic - Quote button - anonymous", function (needs) { @@ -60,48 +69,60 @@ acceptance("Topic - Quote button - anonymous", function (needs) { share_quote_buttons: "twitter|email", }); - skip("Shows quote share buttons with the right site settings", async function (assert) { - await visit("/t/internationalization-localization/280"); - await selectText("#post_5 blockquote"); + chromeTest( + "Shows quote share buttons with the right site settings", + async function (assert) { + await visit("/t/internationalization-localization/280"); + await selectText("#post_5 blockquote"); - assert.ok(queryAll(".quote-sharing"), "it shows the quote sharing options"); - assert.ok( - exists(`.quote-sharing .btn[title='${I18n.t("share.twitter")}']`), - "it includes the twitter share button" - ); - assert.ok( - exists(`.quote-sharing .btn[title='${I18n.t("share.email")}']`), - "it includes the email share button" - ); - assert.ok(!exists(".insert-quote"), "it does not show the quote button"); - }); + assert.ok( + queryAll(".quote-sharing"), + "it shows the quote sharing options" + ); + assert.ok( + exists(`.quote-sharing .btn[title='${I18n.t("share.twitter")}']`), + "it includes the twitter share button" + ); + assert.ok( + exists(`.quote-sharing .btn[title='${I18n.t("share.email")}']`), + "it includes the email share button" + ); + assert.ok(!exists(".insert-quote"), "it does not show the quote button"); + } + ); - skip("Shows single share button when site setting only has one item", async function (assert) { - this.siteSettings.share_quote_buttons = "twitter"; + chromeTest( + "Shows single share button when site setting only has one item", + async function (assert) { + this.siteSettings.share_quote_buttons = "twitter"; - await visit("/t/internationalization-localization/280"); - await selectText("#post_5 blockquote"); + await visit("/t/internationalization-localization/280"); + await selectText("#post_5 blockquote"); - assert.ok(exists(".quote-sharing"), "it shows the quote sharing options"); - assert.ok( - exists(`.quote-sharing .btn[title='${I18n.t("share.twitter")}']`), - "it includes the twitter share button" - ); - assert.ok( - !exists(".quote-share-label"), - "it does not show the Share label" - ); - }); + assert.ok(exists(".quote-sharing"), "it shows the quote sharing options"); + assert.ok( + exists(`.quote-sharing .btn[title='${I18n.t("share.twitter")}']`), + "it includes the twitter share button" + ); + assert.ok( + !exists(".quote-share-label"), + "it does not show the Share label" + ); + } + ); - skip("Shows nothing when visibility is disabled", async function (assert) { - this.siteSettings.share_quote_visibility = "none"; + chromeTest( + "Shows nothing when visibility is disabled", + async function (assert) { + this.siteSettings.share_quote_visibility = "none"; - await visit("/t/internationalization-localization/280"); - await selectText("#post_5 blockquote"); + await visit("/t/internationalization-localization/280"); + await selectText("#post_5 blockquote"); - assert.ok(!exists(".quote-sharing"), "it does not show quote sharing"); - assert.ok(!exists(".insert-quote"), "it does not show the quote button"); - }); + assert.ok(!exists(".quote-sharing"), "it does not show quote sharing"); + assert.ok(!exists(".insert-quote"), "it does not show the quote button"); + } + ); }); acceptance("Topic - Quote button - keyboard shortcut", function (needs) { diff --git a/app/assets/javascripts/discourse/tests/acceptance/unknown-test.js b/app/assets/javascripts/discourse/tests/acceptance/unknown-test.js index a0d15313ad8..b14e9f08df5 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/unknown-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/unknown-test.js @@ -1,6 +1,6 @@ import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers"; import { click, currentURL, visit } from "@ember/test-helpers"; -import { skip, test } from "qunit"; +import { test } from "qunit"; acceptance("Category 404", function (needs) { needs.pretender((server, helper) => { @@ -12,7 +12,7 @@ acceptance("Category 404", function (needs) { }); }); }); - skip("Navigating to a bad category link does not break the router", async function (assert) { + test("Navigating to a bad category link does not break the router", async function (assert) { await visit("/t/internationalization-localization/280"); await click('[data-for-test="category-404"]'); diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js index 336b6e24912..340340ad9d8 100644 --- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js +++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js @@ -12,7 +12,7 @@ import { import { forceMobile, resetMobile } from "discourse/lib/mobile"; import { getApplication, getContext, settled } from "@ember/test-helpers"; import { getOwner } from "discourse-common/lib/get-owner"; -import { later, run } from "@ember/runloop"; +import { run } from "@ember/runloop"; import { setupApplicationTest } from "ember-qunit"; import { Promise } from "rsvp"; import Site from "discourse/models/site"; @@ -434,16 +434,6 @@ QUnit.assert.containsInstance = function (collection, klass, message) { }); }; -export function waitFor(assert, callback, timeout) { - timeout = timeout || 500; - - const done = assert.async(); - later(() => { - callback(); - done(); - }, timeout); -} - export async function selectDate(selector, date) { return new Promise((resolve) => { const elem = document.querySelector(selector); diff --git a/app/assets/javascripts/discourse/tests/integration/components/ace-editor-test.js b/app/assets/javascripts/discourse/tests/integration/components/ace-editor-test.js index 1bdea3ddca3..0e2cc1eaad1 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/ace-editor-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/ace-editor-test.js @@ -12,50 +12,36 @@ discourseModule("Integration | Component | ace-editor", function (hooks) { setupRenderingTest(hooks); componentTest("css editor", { - skip: true, template: hbs`{{ace-editor mode="css"}}`, test(assert) { - assert.expect(1); assert.ok(exists(".ace_editor"), "it renders the ace editor"); }, }); componentTest("html editor", { - skip: true, template: hbs`{{ace-editor mode="html" content="wat"}}`, test(assert) { - assert.expect(1); assert.ok(exists(".ace_editor"), "it renders the ace editor"); }, }); componentTest("sql editor", { - skip: true, template: hbs`{{ace-editor mode="sql" content="SELECT * FROM users"}}`, test(assert) { - assert.expect(1); assert.ok(exists(".ace_editor"), "it renders the ace editor"); }, }); componentTest("disabled editor", { - skip: true, template: hbs` {{ace-editor mode="sql" content="SELECT * FROM users" disabled=true}} `, test(assert) { - const $ace = queryAll(".ace_editor"); - assert.expect(3); - assert.ok($ace.length, "it renders the ace editor"); - assert.strictEqual( - $ace.parent().data().editor.getReadOnly(), - true, - "it sets ACE to read-only mode" - ); - assert.strictEqual( - $ace.parent().attr("data-disabled"), - "true", - "ACE wrapper has `data-disabled` attribute set to true" + assert.ok(exists(".ace_editor"), "it renders the ace editor"); + assert.equal( + queryAll(".ace-wrapper[data-disabled]").length, + 1, + "it has a data-disabled attr" ); }, }); diff --git a/app/assets/javascripts/discourse/tests/integration/components/value-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/value-list-test.js index 46a8d1e207f..6ad51806b6f 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/value-list-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/value-list-test.js @@ -16,8 +16,6 @@ discourseModule("Integration | Component | value-list", function (hooks) { componentTest("adding a value", { template: hbs`{{value-list values=values}}`, - skip: true, - beforeEach() { this.set("values", "vinkas\nosama"); }, @@ -132,8 +130,6 @@ discourseModule("Integration | Component | value-list", function (hooks) { this.set("values", "vinkas|osama"); }, - skip: true, - async test(assert) { await selectKit().expand(); await selectKit().fillInFilter("eviltrout"); diff --git a/app/assets/javascripts/discourse/tests/integration/widgets/home-logo-test.js b/app/assets/javascripts/discourse/tests/integration/widgets/home-logo-test.js index 515dcd85617..9582b3a590f 100644 --- a/app/assets/javascripts/discourse/tests/integration/widgets/home-logo-test.js +++ b/app/assets/javascripts/discourse/tests/integration/widgets/home-logo-test.js @@ -24,7 +24,6 @@ discourseModule( componentTest("basics", { template: hbs`{{mount-widget widget="home-logo" args=args}}`, - skip: true, beforeEach() { this.siteSettings.site_logo_url = bigLogo; this.siteSettings.site_logo_small_url = smallLogo; diff --git a/app/assets/javascripts/discourse/tests/unit/lib/click-track-edit-history-test.js b/app/assets/javascripts/discourse/tests/unit/lib/click-track-edit-history-test.js deleted file mode 100644 index ae13699e88d..00000000000 --- a/app/assets/javascripts/discourse/tests/unit/lib/click-track-edit-history-test.js +++ /dev/null @@ -1,107 +0,0 @@ -import { fixture, logIn } from "discourse/tests/helpers/qunit-helpers"; -import { module, skip } from "qunit"; -import ClickTrack from "discourse/lib/click-track"; -import DiscourseURL from "discourse/lib/url"; -import User from "discourse/models/user"; -import pretender from "discourse/tests/helpers/create-pretender"; -import sinon from "sinon"; - -const track = ClickTrack.trackClick; - -function generateClickEventOn(selector) { - return $.Event("click", { currentTarget: fixture(selector) }); -} - -module("Unit | Utility | click-track-edit-history", function (hooks) { - hooks.beforeEach(function () { - logIn(); - - let win = { focus: function () {} }; - sinon.stub(window, "open").returns(win); - sinon.stub(win, "focus"); - - sinon.stub(DiscourseURL, "routeTo"); - sinon.stub(DiscourseURL, "redirectTo"); - - sessionStorage.clear(); - - fixture().innerHTML = `
-
-
-
-
- google.com - google.com -
- google.com1 - google.com1 -
- google.com - forum - log.txt - #hashtag -
-
- google.com - google.com -
- google.com1 - google.com1 -
- google.com - forum - log.txt - #hashtag -
-
-
`; - }); - - skip("tracks internal URLs", async function (assert) { - assert.expect(2); - sinon.stub(DiscourseURL, "origin").returns("http://discuss.domain.com"); - - const done = assert.async(); - pretender.post("/clicks/track", (request) => { - assert.strictEqual( - request.requestBody, - "url=http%3A%2F%2Fdiscuss.domain.com&post_id=42&topic_id=1337" - ); - done(); - }); - - assert.notOk(track(generateClickEventOn("#same-site"))); - }); - - skip("tracks external URLs", async function (assert) { - assert.expect(2); - - const done = assert.async(); - pretender.post("/clicks/track", (request) => { - assert.strictEqual( - request.requestBody, - "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337" - ); - done(); - }); - - assert.notOk(track(generateClickEventOn("a"))); - }); - - skip("tracks external URLs when opening in another window", async function (assert) { - assert.expect(3); - User.currentProp("external_links_in_new_tab", true); - - const done = assert.async(); - pretender.post("/clicks/track", (request) => { - assert.strictEqual( - request.requestBody, - "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337" - ); - done(); - }); - - assert.notOk(track(generateClickEventOn("a"))); - assert.ok(window.open.calledWith("http://www.google.com", "_blank")); - }); -}); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/click-track-profile-page-test.js b/app/assets/javascripts/discourse/tests/unit/lib/click-track-profile-page-test.js deleted file mode 100644 index 8373b517103..00000000000 --- a/app/assets/javascripts/discourse/tests/unit/lib/click-track-profile-page-test.js +++ /dev/null @@ -1,98 +0,0 @@ -import { fixture, logIn } from "discourse/tests/helpers/qunit-helpers"; -import { module, skip } from "qunit"; -import ClickTrack from "discourse/lib/click-track"; -import DiscourseURL from "discourse/lib/url"; -import pretender from "discourse/tests/helpers/create-pretender"; -import sinon from "sinon"; - -const track = ClickTrack.trackClick; - -function generateClickEventOn(selector) { - return $.Event("click", { currentTarget: fixture(selector) }); -} - -module("Unit | Utility | click-track-profile-page", function (hooks) { - hooks.beforeEach(function () { - logIn(); - - let win = { focus: function () {} }; - sinon.stub(window, "open").returns(win); - sinon.stub(win, "focus"); - - sinon.stub(DiscourseURL, "routeTo"); - sinon.stub(DiscourseURL, "redirectTo"); - - sessionStorage.clear(); - - fixture().innerHTML = `

- google.com - google.com -

- google.com1 - google.com1 -
- google.com - forum - log.txt - #hashtag -

-

- google.com - google.com -

- google.com1 - google.com1 -
- google.com - forum - log.txt - #hashtag -

`; - }); - - skip("tracks internal URLs", async function (assert) { - assert.expect(2); - sinon.stub(DiscourseURL, "origin").returns("http://discuss.domain.com"); - - const done = assert.async(); - pretender.post("/clicks/track", (request) => { - assert.strictEqual( - request.requestBody, - "url=http%3A%2F%2Fdiscuss.domain.com" - ); - done(); - }); - - assert.notOk(track(generateClickEventOn("#same-site"))); - }); - - skip("tracks external URLs", async function (assert) { - assert.expect(2); - - const done = assert.async(); - pretender.post("/clicks/track", (request) => { - assert.strictEqual( - request.requestBody, - "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337" - ); - done(); - }); - - assert.notOk(track(generateClickEventOn("a"))); - }); - - skip("tracks external URLs in other posts", async function (assert) { - assert.expect(2); - - const done = assert.async(); - pretender.post("/clicks/track", (request) => { - assert.strictEqual( - request.requestBody, - "url=http%3A%2F%2Fwww.google.com&post_id=24&topic_id=7331" - ); - done(); - }); - - assert.notOk(track(generateClickEventOn(".second a"))); - }); -}); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/load-script-test.js b/app/assets/javascripts/discourse/tests/unit/lib/load-script-test.js index f977a03ad98..fe48fbc8584 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/load-script-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/load-script-test.js @@ -1,23 +1,8 @@ -import { cacheBuster, loadScript } from "discourse/lib/load-script"; -import { module, skip, test } from "qunit"; +import { cacheBuster } from "discourse/lib/load-script"; +import { module, test } from "qunit"; import { PUBLIC_JS_VERSIONS as jsVersions } from "discourse/lib/public-js-versions"; module("Unit | Utility | load-script", function () { - skip("load with a script tag, and callbacks are only executed after script is loaded", async function (assert) { - assert.ok( - typeof window.ace === "undefined", - "ensures ace is not previously loaded" - ); - - const src = "/javascripts/ace/ace.js"; - - await loadScript(src); - assert.ok( - typeof window.ace !== "undefined", - "callbacks should only be executed after the script has fully loaded" - ); - }); - test("works when a value is not present", function (assert) { assert.strictEqual( cacheBuster("/javascripts/my-script.js"), diff --git a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js index 2f80646e341..8e4bda3cd55 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js @@ -3,7 +3,7 @@ import { applyCachedInlineOnebox, deleteCachedInlineOnebox, } from "pretty-text/inline-oneboxer"; -import QUnit, { module, skip, test } from "qunit"; +import QUnit, { module, test } from "qunit"; import Post from "discourse/models/post"; import { buildQuote } from "discourse/lib/quote"; import { deepMerge } from "discourse-common/lib/object"; @@ -54,20 +54,6 @@ QUnit.assert.cookedPara = function (input, expected, message) { }; module("Unit | Utility | pretty-text", function () { - skip("Pending Engine fixes and spec fixes", function (assert) { - assert.cooked( - "Derpy: http://derp.com?_test_=1", - '

Derpy: http://derp.com?_test_=1

', - "works with underscores in urls" - ); - - assert.cooked( - "**a*_b**", - "

a*_b

", - "allows for characters within bold" - ); - }); - test("buildOptions", function (assert) { assert.ok( buildOptions({ siteSettings: { enable_emoji: true } }).discourse.features diff --git a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js index edbc2ddab24..ddca5c55785 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js @@ -15,7 +15,7 @@ import { slugify, toAsciiPrintable, } from "discourse/lib/utilities"; -import { skip, test } from "qunit"; +import { test } from "qunit"; import Handlebars from "handlebars"; import { discourseModule } from "discourse/tests/helpers/qunit-helpers"; @@ -282,21 +282,4 @@ discourseModule("Unit | Utilities", function () { } }); }); - - skip("inCodeBlock - runs fast", function (assert) { - const phrase = "Lorem ipsum dolor sit amet, consectetur adipiscing elit."; - const text = `${phrase}\n\n\`\`\`\n${phrase}\n\`\`\`\n\n${phrase}\n\n\`${phrase}\n${phrase}\n\n${phrase}\n\n[code]\n${phrase}\n[/code]\n\n${phrase}\n\n ${phrase}\n\n\`${phrase}\`\n\n${phrase}`; - - let time = Number.MAX_VALUE; - for (let i = 0; i < 10; ++i) { - const start = performance.now(); - inCodeBlock(text, text.length); - const end = performance.now(); - time = Math.min(time, end - start); - } - - // This runs in 'keyUp' event handler so it should run as fast as - // possible. It should take less than 1ms for the test text. - assert.ok(time < 10); - }); });