Skip to content

Commit

Permalink
Fix sync-tag eslint rule (#647)
Browse files Browse the repository at this point in the history
## Summary:
The tests didn't catch this because they were mocking it.  The reason why we need to bother with these util wrappers around functions is that RuleTester doesn't provide a way to mock things so I ended up having to mock things in a hacky way.  I added an assert() call to the place where we do the mocking, but I think we need rethink how we write eslint tests.  We should probably create our own jest-based test harness, but that'll take some doing so I'm going to punt on it for now.

Issue: None

## Test plan:
- yarn --cwd packages/eslint-plugin-khan test

Author: kevinbarabash

Reviewers: jeresig, kevinbarabash

Required Reviewers:

Approved By: jeresig

Checks: ✅ codecov/project, ✅ Test (macos-latest, 16.x), ✅ CodeQL, ✅ Lint, typecheck, and coverage check (ubuntu-latest, 16.x), ⏭  dependabot, ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ Analyze (javascript)

Pull Request URL: #647
  • Loading branch information
kevinbarabash authored Apr 14, 2023
1 parent 4f5821e commit a621be6
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/eight-bats-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/eslint-plugin": patch
---

Ensure sync-tag rule has access to 'execSync'
3 changes: 2 additions & 1 deletion packages/eslint-plugin-khan/lib/util.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const {execFile} = require("child_process");
const {execFile, execSync} = require("child_process");

// This is done so that we can override execSync in the tests
module.exports = {
execFile,
execSync,
};
3 changes: 3 additions & 0 deletions packages/eslint-plugin-khan/test/sync-tag_test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const assert = require("assert");

const {rules} = require("../lib/index.js");
const util = require("../lib/util.js");
const RuleTester = require("eslint").RuleTester;
Expand All @@ -9,6 +11,7 @@ const parserOptions = {
const ruleTester = new RuleTester(parserOptions);
const rule = rules["sync-tag"];

assert(util.execSync);
util.execSync = (command) => {
console.log("execSync mock --------");
if (command.includes("filea")) {
Expand Down

0 comments on commit a621be6

Please sign in to comment.