Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: watch for changed integration test files #464

Merged
merged 9 commits into from
Jan 6, 2022
Merged

Conversation

boukeversteegh
Copy link
Collaborator

@boukeversteegh boukeversteegh commented Jan 6, 2022

This was quite some work, but well worth it.

Added

yarn watch

Watch for changes in integration/*/*.{bin, proto} and automatically run the correct scripts (proto2bin, proto2pbjs and bin2ts).

Automatically keeps .ts and .pbjs files up to date while working on ts-proto.

Motivation

  • Remembering which scripts to run when stayed difficult, and I still messed it up from time to time.
  • Running the scripts on all test files is slow
  • Running the script on a single file is fast, but you need to type commands by hand, or create new profiles in your IDE every time you are working with a different integration test
  • Improve overall developer experience

Features

  • Only processes the changed files

Missing features

  • Does not regenerate ALL test files automatically when you change the plugin implementation (e.g. main.ts).
    This would be extremely impractical as it takes at least a minute, and would run each time you type a letter

Implementation

  • Uses chokidar to watch for file changes
    • This is the library used internally by nodemon
    • nodemon cannot pass the changed files as arguments to the configured exec script, so using nodemon would force us to regenerate all files, each time one changed.
    • npm-watch is a nice wrapper around nodemon but has the same limitation, so couldn't be used
  • All generation shell scripts had to be adapted:
    • Accepting an input file: i.e. codegen.sh value/value.bin (this is passed by yarn watch)
    • Keep backwards compatibility:
      • Running without arguments generates files for all tests
      • Specifying a directory name to only generate files for that test
  • integration/pbjs.sh
    • Still "by hand", no abstraction was introduced
    • Comments replaced by if statements to allow for partial execution
    • If new integration tests are added (i.e. from another PR) without the if statement, the script will still work, but execute that item unconditionally

Screenshot

Running yarn watch and updating integration/simple-optionals/simple.proto

image

Copy link
Owner

@stephenh stephenh left a comment

Choose a reason for hiding this comment

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

Wow, this is amazing! I'm surprised at how small & neat the watch.ts script is too; I'm definitely going to be cribbing this for another use cases I have on other/future projects. :-D

Thanks!

package.json Outdated Show resolved Hide resolved
@boukeversteegh
Copy link
Collaborator Author

Wow, this is amazing! I'm surprised at how small & neat the watch.ts script is too; I'm definitely going to be cribbing this for another use cases I have on other/future projects. :-D

Thanks!

I'm glad to hear that! I see that this doesn't work on Ubuntu in WSL, so I'm adding an option --polling to fix that.

Please let me know if it works on Linux.

Then I see there's still an issue generating the pbjs files, so I broke something there. Will fix that.

@stephenh
Copy link
Owner

stephenh commented Jan 6, 2022

@boukeversteegh wow, worked the 1st time on linux. docker-compose run even auto-built the images on the 1st run, and then just ran them directly on subsequent saves. Looks great!

@boukeversteegh boukeversteegh merged commit 988cd7e into main Jan 6, 2022
@boukeversteegh boukeversteegh deleted the feature/watch branch January 6, 2022 20:41
stephenh pushed a commit that referenced this pull request Jan 6, 2022
# [1.98.0](v1.97.2...v1.98.0) (2022-01-06)

### Features

* watch for changed integration test files ([#464](#464)) ([988cd7e](988cd7e))
@stephenh
Copy link
Owner

stephenh commented Jan 6, 2022

🎉 This PR is included in version 1.98.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants