test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

test: replace requireInject with t.mock tap api - #2370

Closed
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock
Closed

test: replace requireInject with t.mock tap api #2370
ruyadorno wants to merge 2 commits into
npm:latestfrom
ruyadorno:replace-require-inject-with-tap-mock

Conversation

@ruyadorno

@ruyadornoruyadorno commented Dec 17, 2020

Copy link
Copy Markdown
Contributor
  • Replaces all usage of requireInject with t.mock
  • Standardizes usage of tap as: const t = require('tap')
  • Fixes some rare cases in which mocks were relying in the fact that
    requireInject placed mocks within the require.cache and made mocks
    available for nested files, this type of scenario now requires an extra
    t.mock declaration in order to define mocks to that nested file.
  • Requires a new release of tap, since currently this PR is just relying in a fork to run all tests, ref: Add mock API tapjs/tapjs#698

@ruyadorno
ruyadorno requested a review from a team as a code ownerDecember 17, 2020 05:26
@darcyclarkedarcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Dec 17, 2020
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch 2 times, most recently from 8c01f81 to c50be61CompareDecember 17, 2020 19:15
- Replaces all usage of `requireInject` with `t.mock`
- Standardizes usage of tap as: `const t = require('tap')`
- Fixes some minor inconsistencies in tests
@ruyadorno
ruyadornoforce-pushed the replace-require-inject-with-tap-mock branch from c50be61 to 8742c17CompareDecember 17, 2020 23:07
@ruyadornoruyadorno mentioned this pull request Dec 17, 2020

@darcyclarkedarcyclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocked: Need to land the change in tap first & then update the dep in package.json

@darcyclarkedarcyclarke removed the release: next These items should be addressed in the next release label Dec 18, 2020
@darcyclarkedarcyclarke added Needs Review semver:patch semver patch level for changes labels Jan 6, 2021
@darcyclarke

Copy link
Copy Markdown
Contributor

@ruyadorno we need to land this. @isaacs I'm willing to commit resources to get this across the finish line next week, unless you've got any reservations about landing it. I know It probably means cutting a minor of tap but I don't know what the blocker has been other than just getting time to do that - maybe we can get more resources dedicated to tap moving forward(?) ~ especially given that we rely on it so heavily throughout the CLI

@darcyclarkedarcyclarke added the release: next These items should be addressed in the next release label Feb 13, 2021
@ruyadornoruyadorno removed the release: next These items should be addressed in the next release label Feb 25, 2021
@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

We need to update to tap@15 (not yet released at this point in time) and do a second pass of updating all test files, fixing conflicts and migrating new instances of require-inject usage.

I'm removing the Release-Next label to better reflect the current state.

@wraithgar

Copy link
Copy Markdown
Contributor

Woohoo! This got done in #3069

@ruyadorno

Copy link
Copy Markdown
ContributorAuthor

🥳

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 7.xwork is associated with a specific npm 7 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@darcyclarke@wraithgar