Create gulp task to facilitate TDD

[andrewconnell]

Currently developers must manually run builds of the library, manually vet code & manually run tests. Create a gulp task(s) that will watch for code changes and automate the above stuff.

[andrewconnell]

Putting this back in the pool. It's needed and I won't get to it for a few days due to other work. So, if someone wants to tackle it between now & then, go for it.

The idea here is that you can create a new gulp task... something like gulp live-dev that does the following:

• watch for changes to any *.ts, *.js or *.html file... if anything changes...
• run all tests & vetting: 'gulp vet' & 'gulp test'
  • note there seems to be an issue in the test site where karma doesn't release... it seems to hang and you have to manually kill it with CTRL+C... this will hold up additional work...
• rebuild the library in debug mode: `gulp build-lib'
• support all the existing arguments --verbose, `--specs', '--debug'... the first one should list out the file that triggered the rerun

anything else i missed?

[s-KaiNet]

I can do that for a day or two. If that acceptable estimate I'm ready to go)

[andrewconnell]

thx!

[andrewconnell]

I'm using this new watcher when working on the uif-button and have run into a few things...

• auto testing on build: This change introduced a dependency on the build-lib task in that it runs test prior to building. If any tests fail, it won't build. If you are doing TDD this means you have to code to pass ALL tests before you can see anything working in your demo. But if you remove this precondition, the watcher won't run the tests because it only watches for changes to the tests to trigger running tests.

  Scenario: With uif-button I wrote tests first to test against the spec I'm working against, 22 tests in total. I also created a demo page so I can see if the renderings are correct or if I missed something with a test. As it sits now... I have to code the directive to pass all tests before I can preview how it looks.

Locally I've made a few changes (local only)... curious on opinions:

• in task build-lib, remove the dependency on the task test
• in task watch, for the last watcher, added ./src/**/*.ts to the files to watch & added the task vet before running test

This way the first watcher will always build the library on any test / src change & the last one will always vet & test everything on the same condition. Merge these two watchers?

Thoughts?

Would also be nice to add a browsersync option to watch for changes in the built ./dist/*.js & ./src/**/*.html to reload the browser automatically. Thoughts?

[s-KaiNet]

Agreed on both points.
But I'd rather add transpile-ts as dependency for the test task, because tests require .js files and transpile-ts in its turn depends on vet.
Also thinking about something like this - gulp watch --dev --debug - in this case karma should start with option singleRun=false and additional browser param for debugging tests in chrome.

[andrewconnell]

Works for me... that's another thing I noticed. When you startup watch it waits... ideally it should do the first pass on everything.

[s-KaiNet]

Hmm.. do you know how to do that? Normally gulp watcher don't start anything on initial watch.

[andrewconnell]

Just a dependency on the watch task itself?

[s-KaiNet]

Yes! The answer is extremely simple :). Thank you! Never think in that way, that watch can have deps.

[s-KaiNet]

Started implementation.

[s-KaiNet]

Question: do we really need dependency on watch?
Scenario: I'm working on directive, made a lot of changes, tests are not finished and still failing. Closed cmd because it crashed\reboot\need to go\whatever reason. Later I back to work, run gulp watch and it fails because I still has unfinished tests. In this case task (test) which is dependent on watch is failing and that prevents watchers from starting. This prevents me to start watchers until I fix all tests.
Is it better to live it as is? No dependencies for watch, if you need to run build before watch, then you need explicitly run gulp build-lib

[andrewconnell]

From my POV, when I run a watcher like this, I want it to do everything (vet, run tests, build library) without any blockers… it would do all that, report errors & then go back to watching.

Our gulp tasks SHOULD be written today to log the errors, not fail… if not, then IMHO that’s a bug.

So with this thinking I don’t think having dependency in place is bad… calling watch would run everything the first time, then just watch to see what it needed to do from there. Thoughts?

[s-KaiNet]

From my POV, when I run a watcher like this, I want it to do everything (vet, run tests, build library) without any blockers… it would do all that, report errors & then go back to watching.

Agreed. This is how it's working right now. The problem is, when running everything first time before watch and there is an error (at least one, vet error, or failing test), it prevents other task to run (watch task itself). Because of this we can't start watchers at all.
Let me illustrate this into following workflow. We added test as dependency on watch (test in turns depends on transpile-ts and transpile on vet). On first run, right after gulp watch, following will occur:
vet -> transpile -> test -> watch. If vet fails, then watch won't run, if test fails, watch won't run, etc. The process simply exits because preceding tasks are failed.
The question is how to deal with that? I'm not sure how to handle this through the gulp.
Do you know any workaround or practice for that?

[andrewconnell]

Ah now I see what you're saying... Offhand I don't know. What if instead of within the task instead of the dependency it triggers the first three tasks and when they all finish it then sets of the watchers. Maybe using the module run-sequence?

[s-KaiNet]

Yep, sorry, my first explanation was not so clear.
run-sequence should work, will try that, thanks!

[ghost]

Hi, good job @s-KaiNet ! However, I don't like that it stops when my vet test fails. E.g. in development I don't worry too much about vet issues, I fix those at the end. I'm interested to know if my tests fail or pass.
I think part of your changes "gulp test" now also fails when vet fails.
So yes, I think run-sequence is the way to go.

And again good stuff, this will make our lifes easier :)

[s-KaiNet]

Thank you @rolandoldengarm !!!
I understand you issue, will make a change to support you scenario too.
How it looks like:
gulp watch - on first start vet, test, build on a sequence, if any errors - just continue
On any change *.ts under sources vet, test, build run on a sequence, if any errors - continue, no more dependencies
On any change *.ts under demo folder - transpile
On any change *.ts under build files - transpile

good?

[andrewconnell]

That looks good... one day would be cool if we got to the point where we could also use something like browsersync to have the demo page up and we can see the components working visually with auto refreshes... but that's down the road...

[s-KaiNet]

Good news @andrewconnell
That's already done! Just need to do final tests for all of this all together and PR will be on the way.

[s-KaiNet]

Also what I'm thinking about. When we run test task, it runs all tests, but in case of watching we probably need to run only one .spec.ts file from our directory where we are working. For example I have following issue: I'm working on directive under components/contextualmenu, I saved file and triggers test task in progress. Currently there are more than 140 tests and it produces a really long output, I need to scroll up to find which exact test failed and why. Also it takes more time to run all 140 tests instead of only 10-20 which a related to my directive.
What I'm proposing:
If watch detects that .spec.ts file changed - then run only this particular test.
If watch detects that .ts (say file components/contextualmenu/contextualmenu.ts) file changed - then find in current directory (contextualmenu) corresponding .spec.ts file (contextualmenu.spec.ts) and run only this particular test.
Benefits - faster workflow, significantly reduced output for test task during watch, test output contains only tests related to directive (folder) you are working on.
All this is relevant to watch task only, regular run gulp test will do the test for all of the directives.

Does that make sense?

[andrewconnell]

I’d personally love this capability… specifically having a way to filter to a specific spec file. If the watcher can detect and only run that test, that would be ideal.

Would be nice if we also added this as an option to the basic test task to… something like gulp test —filter:table. That doesn’t have to be part of this update, but something to think about…

[s-KaiNet]

Thank you guys for your feedback. My first attempt didn't take all cases into account, but now with your help I believe "watch" story looks much better.
Short description:

gulp watch

On first run (before watchers setup) executes vet, transpile, build-lib, test in a sequence. Any errors just reported into the console, no blocking.
When watching:
[source or spec changed] -> runs vet, transpile, build, test in a sequence, any errors also are reported and also no blocking. For tests it grabs only spec files under folder you are working on, i.e. if src/components/icon/icon.ts changed, then test will run only specs which are in the folder src/components/icon. NOTE - I assume that all source and spec files located under src/components/<directive>/. If in future we will have some source or spec files under nested folders (src/componets/<directive>/source/file.ts), then watcher needs to be changed to address this
[build or demo files changed] -> vet and transpile only

gulp browser-sync

Runs browser-sync in a static mode with default port of 3000. After run it opens browser window with url pointed to http://localhost:3000/. After that you need to open your demo by using mentioned url, such as http://localhost:3000/src/components/icon/demo/index.html, you will see a message at the top right corner Connected to BrowserSync. Now changes in demo assets will trigger browser refresh.

gulp test --watch

Allows you to debug test in Chrome and automatically watch on file changes.
That's it.

If you want to use all of this tasks at the same time, you need to run it separately, i.e. 3 console instances (or tabs in Conemu). I've tried to put them in one watch and they are totally messed up, because every use it's own watcher.

I did my best in testing for all different situations, but of course there still might be errors\issues\inconsistencies. You are welcome to report all this and I will try to address.

[ghost]

Cool cool cool, can't wait to check it out.

[andrewconnell]

Very nice work! Love this! Functionality you’ve nailed it… looking forward to using this! I have a few questions / comments… before merging this in I’d like to see what some others say with this. Mostly I’m thinking about the experience of the developer and help them understand what’s going on without much docs.

BTW - I added a few line comments in the PR as well that I think we should address before merging this in so please check those out as well…

Regarding browser-sync -

First the name (yes, I’m being a little picky here)… I think we could rename this to be more descriptive of what this is doing… for the name to speak the intent. Browser sync as a name doesn’t say what this does. What this is doing is doing live reloading of a new server. Maybe “serve-demo” or “static-serve” or just “serve”?

Also, for me, this really should be part of the watcher… not saying we should remove the ability that lets you run it independently… but right now it’s not possible to do live dev (that the new “watch” facilitates) AND see the demo… seems like a gap. The only way around this is two console windows, one kicking off browser-sync, one kicking off watcher. Maybe a switch for “gulp browser-sync —watch” or the other way around (“gulp watch —browser-sync”) <<< these would be pending the name changes I’m proposing…

The description in the task needs to explain that you will have to manually type the URL => demo… I admit that I ran it and was looking at the gulp help description as well as the code and couldn’t figure out why my stuff wasn’t loading. I see it’s mentioned in the gulp task doc, but maybe updating the actual task file & description written in the gulp help?

Regarding watch -

Again, I”m going to be picky, but it’s a naming thing. Maybe “auto-test” or “live-test” or “continuous-dev” (or a shorter version) or “live-dev”? Does the name “watch” work for others? Some project use the word watch, I’ve always preferred task names that speaks more to the intent.

Regarding run-all-sequence -

Same thing with the name… we don’t need to use the module name in the task name. Current name implies every gulp task is run, but that isn’t what it does… What does this do? It runs all the dev stuff… vet, transpire, build & test. Some ideas: “build-check”, “check-build”, “build-all”, “build-full”, “validate-build”. Personally I favor the last one… thoughts?

[s-KaiNet]

Very good points, @andrewconnell , thank you! Agreed with all of them. I was thinking from my own view and definitely miss the point that all of this should be obvious and intuitive for others (especially for those who wants to quickly jump-in to the project). The only thing which make me sad, is that both watch and browser-sync don't want to work well together for me. The process exit with error, but I will give them another shot, may be I did something wrong.
For the memory:

• rename browser-sync to serve-demo (instead flag serve to live-dev added)
• combine serve-demo and watch (or try at least)
• make description for serve-demo more informative and descriptive
• rename watch to live-dev
• rename run-all-sequence to validate-build
• address issues commented out in the PR

I believe your points look good for the team and it doesn't make big sense to wait response from other guys. This task is rather helpful and wanted, so I'm just trying to speed up all the things. I'm starting this changes, and if any comments\thoughts etc. will address them.

Agreed?

[andrewconnell]

Whew… glad you took my feedback like like that… you did great work and didn’t want to come across wrong :)

Everything looks good. As for the combination of serve-demo & watch… maybe it’s just a flag? We do want two separate tasks because maybe someone is just working on demos while someone else wants to work on the directive and look at the demos live. At any rate, the way you have it supports both of these scenarios, the user would just have to open a second console window to run the browser sync while watching happens in one. I think that’s perfectly acceptable for now.

If adding the option to the watcher to startup browser sync isn’t a simple thing like you say, then remove that from your checklist. Two console windows is acceptable IMHO.