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.

Sign up for GitHub

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

Jump to bottom

React tests #188

Merged
merged 2 commits into from
Oct 28, 2015
Merged

React tests #188

merged 2 commits into from
Oct 28, 2015

Commits on Oct 27, 2015

  1. react: factor out assertCellValue 

    The motivation is twofold:
First, to reduce repetition in the code.
Second, to provide the user the ability to see the expected value of the
cell versus the observed value so the user has a better idea of what
went wrong.
    @petertseng
    petertseng committed Oct 27, 2015
    Configuration menu
    Copy the full SHA
    17c1055 View commit details
    Browse the repository at this point in the history

  2. react: Add tests with new topologies 

    The added toplogies are all topologies that are not being tested before:

* compute2 depending directly on inputs (instead of on compute1).
  Note that this also tests having multiple inputs, something that was
  not tested before.
* compute1 depending on other compute1 for value. Note that the
  multiple dep changes test has a compute1 depending on compute1... but
  it only tests for the callback of a dependent compute2 and doesn't
  check the value of the dependent compute1, so this test is still new.
* compute2 depending on other compute2.

Why should we care?

It is possible that the limited range of topologies in the existing
tests will drive some solutions to work only for those specific
topologies.

Before this commit, the minimal non-generic solution that would pass
tests is "when an input is updated, update compute1 cells depending on
that input, update compute2 cells depending on those compute1 cells".
Again note that the multiple dep changes test has a compute1 ->
compute1 connection but the value is never checked, so the test still
passes despite this minimal solution not updating it. Knowing this, it
could be somewhat tempting to implement such a solution instead of the
generic solution.

I believe the new tests make clearer that a solution should be generic
across all possible topologies. Clearly we can never *ensure* that with
just tests, but I think this gets us closer than we were before.
Solutions will be more likely to take it into account, and reviewers
are freed from having to explain about topology limitations and can
focus attention on other areas.

Test version is bumped for this.
    @petertseng
    petertseng committed Oct 27, 2015
    Configuration menu
    Copy the full SHA
    c7616dc View commit details
    Browse the repository at this point in the history