Skip to content

Conversation

@joshbax189
Copy link
Contributor

@joshbax189 joshbax189 commented Mar 20, 2025

Closes #308

Test scripts and fixtures are all in test/jest.
I've followed the pattern workflows/<x>.yml becomes test/jest/<x>.test.js and any project files are in test/jest/<x>/.
Most of the project files are either the local files from the original test directories, or renamed copies of mini.pkg.1 from test/fixtures. This is to make sure tests are properly isolated.

  • Run all tests using npm run test.
  • Run an individual test using its path: npm run test ./test/jest/analyze.test.js
  • Run a test with full command output: npm run test-debug ./test/jest/analyze.test.js
  • Always do this from the same directory as package.json (I use relative paths in the tests)

I've added a doco file under Contributing/Testing. It should probably replace the Testing section in the Contributing/Developing Eask page.

Test naming

I've used something like this when the tests are simply "does it work"

analyze          # name of command/group
   in fixture-1   # which CWD is used
      - eask analyze       x   # command name is the test
      - eask analyze --sth x

pretty easy to quickly re-run the test in a shell to work out what's broken.
For example local.test.js

But proper behavior tests probably need a different style

analyze          # name of command/group
   in fixture-1 calling eask analyze   # which CWD and command
      - should analyze an error         x   # description of the test, not the command
      - should ignore a different error x

For example link.test.js

Matrix

These are the existing tests and the versions they run on, per the workflows.

Emacs versions: 26.3, 27.2, 28.2, 29.4, 30.1, snap
OS: linux (ubuntu), win, mac

Test Emacs OS Exclude Notes
analyze * * mac/26, mac/27
color 30.1 * previously deprecated
config * * mac/26, mac/27
docker snapshot linux might be working on mac now
emacs * * mac/26, mac/27
error 30.1 * previously deprecated
exec 27-snap * mac/27 windows command error in jest
exit_status 30.1 *
global * * mac/26, mac/27
install * * mac/26, mac/27
link 27-snap * mac/27
local * * mac/26, mac/27
options 30.1 *
outdated_upgrade 30.1 *
search 30.1 *
test_buttercup 30.1 *
test_ecukes 30.1 *
test_ert-runner 30.1 *
test_ert 30.1 *
upgrade-eask 30.1 * new test

Notes

  • for now I'm using CJS modules, as that is jest's default
    can use ESM too if you'd like
  • env var ALLOW_UNSAFE allows tests that modify global or config files.
    Use testUnsafe() instead of test() or it() for any tests that use --config or --global
  • make sure to await the pattern expect(cmd).rejects.toThrow() or else it will give a false positive!
  • an error spawn /bin/sh ENOENT means that the cwd path doesn't exist, check it is correct relative to project root
  • I use tide and npm i --no-save @types/jest to improve local development.
  • use EASK_COMMAND=$PWD/bin/eask to use the local version of eask, e.g.
    env EASK_COMMAND=$PWD/bin/eask npm run test

TODOs

  • write up doco
    • testing patterns, e.g. success, error, match output, match snapshot, check files
    • common problems
    • env vars and npm scripts
    • inline doco for helper methods
  • add back the workflows with updated commands
  • should we remove old tests and makefile targets?
  • fix timeout issues, either optimize or increase timeouts (won't fix now)
    • seems to be caused by recipes in dependencies
    • local.test.js times out first time building temporary archives
    • docker.test.js times out first time pulling image
    • these don't seem to be a problem in CI, can just manually increase local timeout
  • check tests for ordering dependencies: e.g. when install-deps should be in a beforeAll rather than a test
  • move tests to test/js
  • provide examples of snapshot testing and output matching
    see analyze.test.js
  • finish porting remaining tests
  • allow switching eask and ./bin/eask in commands, likely with a command helper runEask("install-deps")
  • collect boilerplate in, e.g., a class
    see TestContext in ./helpers.js and example in exec.test.js

@github-actions github-actions bot added the CI label Mar 20, 2025
@joshbax189
Copy link
Contributor Author

Looks like I made a typo in one of the eask commands. Good to see npm runs on all OS's and produces a similar error in each case.

@jcs090218
Copy link
Member

jcs090218 commented Mar 20, 2025

Looking good so far! 🚀 😉

Can jest print the full output? It's really useful when I want to quickly see the command's output. 🤔 I found the --verbose option, but it only prints the test descriptions.

Test scripts and fixtures are all in test-js.

I prefer keeping everything under the test folder, maybe in test/js/. I've seen a few repos follow this convention, but I'm not sure if it should be encouraged (having multiple test-xxx folders). 🤔

pretty easy to quickly re-run the test in a shell to work out what's broken.

But proper behavior tests probably need a different style

👍 Yeah, my test was pretty simple and just focused on what works and what doesn’t.

for now I'm using CJS modules, as that is jest's default
can use ESM too if you'd like

CJS is fine since ESM is not supported in yao-pkg... yet? See #291.


Edit: Could you add some documentation? It would be helpful to have details on things like this! 😁

@joshbax189
Copy link
Contributor Author

Have cut down the CI jobs for now. I put the ones that I plan to merge into Jest Tests, but haven't yet, in the deprecated folder.
Later I'll re-add the matrix entries for Jest Tests.

@joshbax189
Copy link
Contributor Author

Can jest print the full output? It's really useful when I want to quickly see the command's output.

That's a really useful idea! I've added the npm run test-debug command. The output is not exactly the same, since jest insists on marking up the console.log calls, but hopefully it's serviceable.

I prefer keeping everything under the test folder, maybe in test/js/.

Sweet, I'll move them there.

Could you add some documentation? It would be helpful to have details on things like this! 😁

Absolutely. I'll add comprehensive doco perhaps more at the end of the PR when things stabilize, just to avoid revising it a lot.
Definitely flag any things that are unclear and I'll clarify them sooner.

@joshbax189 joshbax189 force-pushed the feat/js-test-framework branch 3 times, most recently from ec0a8a1 to 6f8eeea Compare March 25, 2025 04:55
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 25, 2025
@joshbax189 joshbax189 force-pushed the feat/js-test-framework branch 2 times, most recently from 14b893c to 5dd3782 Compare March 26, 2025 06:52
@joshbax189 joshbax189 force-pushed the feat/js-test-framework branch from 32099e6 to 597db81 Compare March 26, 2025 19:12
@joshbax189 joshbax189 force-pushed the feat/js-test-framework branch from 63581bb to 949a6fc Compare March 28, 2025 04:25
@joshbax189 joshbax189 force-pushed the feat/js-test-framework branch from 949a6fc to 7f75288 Compare March 28, 2025 04:29
@joshbax189
Copy link
Contributor Author

rebased to remove jest workflow and recover the previous workflows

@joshbax189 joshbax189 force-pushed the feat/js-test-framework branch from 03926c5 to 9d0d3f7 Compare March 28, 2025 05:45
@joshbax189 joshbax189 force-pushed the feat/js-test-framework branch from 9d0d3f7 to e31f0f5 Compare March 28, 2025 06:02
@jcs090218
Copy link
Member

The PR looks ready? Is there something we missed? 😋

@joshbax189 joshbax189 marked this pull request as ready for review March 28, 2025 17:58
- name: Testing...
run: |
make test-buttercup
npm run test-unsafe test/jest/buttercup.test.js
Copy link
Member

Choose a reason for hiding this comment

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

The name should be test-buttercup.test.js? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good spot! renamed it too

- name: Testing...
run: |
make command-outdated-upgrade
npm run test-unsafe test/jest/outdated.test.js
Copy link
Member

Choose a reason for hiding this comment

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

The name should be outdated-upgrade.test.js?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, renamed

@jcs090218 jcs090218 merged commit d8acd81 into emacs-eask:master Mar 29, 2025
177 checks passed
@jcs090218
Copy link
Member

🥳 Thank you very much for this! We finally have the local testing!

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

Labels

CI documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider using a test framework

2 participants