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

Restart #68

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Restart #68

wants to merge 30 commits into from

Conversation

magomimmo
Copy link
Contributor

hi,
here is a pull request which started porting domina test on clojurescript.test as we did in enfocus.
At the moment I did not add the piggieback and the ring/compojure stuff, because I'll add autin later (it's much easier to setup).

  • I moved the old unit test under the temp directory
  • I added the cljsbuild :source-paths pathnames to the leiningen :source-paths and removed the :jar true setting
  • I added the profiles.clj and moved into it all the configuration pertaining the dev profile
  • I create a lot of standard unit tests. The *_standard_test.cljs files contain few of the test from the old tests. the one already implemented have been comment in the original tests in such a way the we know what has to be completed. The *edge_test.clj files contain the unit tests for the edge case.

I did not touch the original code (aside from a very little minutiae), but I think we should think better about the expected result from the edge case. I set the edge tests in such a way that all of them pass against the current code, but there few place where my personal opinion is that we should fix the code. But is something we can do later, when we would have ported all the old tests.

LMK what you think about it. At the moment we can stay with the 1.0.3-SNAPSHOT, because the codebase is not changed.

To test the pull request:

lein compile, test

All the 28 test/138 assertion passed with any optimisation level, but the none (which I do not specify as a test-command).

To run a single test-command (e.g. whitespace)

lein cljsbuild test whitespace

that's all

HIH

@ckirkendall
Copy link
Collaborator

It may take a while for me to merge this in. I need to make sure Luke is
cool with this direction and I need to make sure his automated test setup
works with the new setup. I probably wont get to it until this weekend or
early next week. I am going to concentrate on figuring out the issues with
new cljs versions.

On Wed, Nov 6, 2013 at 5:45 PM, Mimmo Cosenza notifications@git.luolix.topwrote:

hi,
here is a pull request which started porting domina test on
clojurescript.test as we did in enfocus.
At the moment I did not add the piggieback and the ring/compojure stuff,
because I'll add autin later (it's much easier to setup).

  • I moved the old unit test under the temp directory
  • I added the cljsbuild :source-paths pathnames to the leiningen
    :source-paths and removed the :jar true setting
  • I added the profiles.clj and moved into it all the configuration
    pertaining the dev profile
  • I create a lot of standard unit tests. The *_standard_test.cljs
    files contain few of the test from the old tests. the one already
    implemented have been comment in the original tests in such a way the we
    know what has to be completed. The *edge_test.clj files contain the unit
    tests for the edge case.

I did not touch the original code (aside from a very little minutiae), but
I think we should think better about the expected result from the edge
case. I set the edge tests in such a way that all of them pass against the
current code, but there few place where my personal opinion is that we
should fix the code. But is something we can do later, when we would have
ported all the old tests.

LMK what you think about it. At the moment we can stay with the
1.0.3-SNAPSHOT, because the codebase is not changed.

To test the pull request:

lein compile, test

All the 28 test/138 assertion passed with any optimisation level, but the
none (which I do not specify as a test-command).

To run a single test-command (e.g. whitespace)

lein cljsbuild test whitespace

that's all

HIH

You can merge this Pull Request by running

git pull https://github.com/magomimmo/domina restart

Or view, comment on, or merge it at:

#68
Commit Summary

  • move macros.clj to src/clj
  • upgrade clj to 1.5.1, cljs to 0.0-1847, cljsbuild to 0.3.2, remove
    unused stuff
  • remove blank line from the topo
  • remove not existent dispatch-event fn
  • add clojurescript.test and move testing stuff into the dev profile
  • remove dev profile
  • add dev profile
  • move test to temp
  • move from vector to map, added :source-paths
  • name builds
  • add test-commands, dev-resources, remove :ouput-dir and modify
    :output-to
  • comment test implemented with clojurescript.test
  • add unit tests
  • add more unit tests
  • update rules
  • update cljs to r1978 and cljsbuild to 1.0.0-alpha1
  • update edge unit tests

File Changes

  • M .gitignorehttps://github.com/Restart #68/files#diff-0(5)
  • A profiles.cljhttps://github.com/Restart #68/files#diff-1(35)
  • M project.cljhttps://github.com/Restart #68/files#diff-2(48)
  • R src/clj/domina/macros.cljhttps://github.com/Restart #68/files#diff-3(0)
  • M src/cljs/domina/events.cljshttps://github.com/Restart #68/files#diff-4(1)
  • R temp/test/cljs/domina/test.cljshttps://github.com/Restart #68/files#diff-5(48)
  • A test/cljs/domina/css_edge_test.cljshttps://github.com/Restart #68/files#diff-6(20)
  • A test/cljs/domina/css_standard_test.cljshttps://github.com/Restart #68/files#diff-7(31)
  • A test/cljs/domina/fixtures.cljshttps://github.com/Restart #68/files#diff-8(22)
  • A test/cljs/domina/xpath_standard_test.cljshttps://github.com/Restart #68/files#diff-9(31)
  • A test/cljs/domina_edge_test.cljshttps://github.com/Restart #68/files#diff-10(247)
  • A test/cljs/domina_standard_test.cljshttps://github.com/Restart #68/files#diff-11(193)

Patch Links:

@magomimmo
Copy link
Contributor Author

Sure, no problem. Just a suggestion. By introducing the edge cases, you are really thinking about the domain and the co-domain of a function (we love functional programming). I strongly suggest to take a look at the edge cases. At the moment I implemented the expected result of each tested function to be the one implemented in domina, but there are cases in which (like we discovered in enfocus) IMHO that results are wrong and we should fix the code consequently.

The implemented edge cases (at the moment only for few functions) are in the following:

Cheers

@magomimmo
Copy link
Contributor Author

On Nov 7, 2013, at 1:45 AM, Creighton Kirkendall notifications@github.com wrote:

I am going to concentrate on figuring out the issues with
new cljs versions.

At the moment I can say that the issues with new cljs version regards the old tests, not the domina codebase and neither the new colurescript.test unit test lib.

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

Successfully merging this pull request may close these issues.

2 participants