ClojureScript

cljs.import-test not run in test suite / ordering problem in the compiler

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

The test code from CLJS-312 is not hooked up to the test suite, which is trivially fixed.

Hooking up the test, however reveals a deeper problem (besides that it has a failing assumption):

Presumably due to the goog.provides later in the files, the compiler orders the output wrong, which results in not provided errors.
Most interestingly, this problem goes away, when compiling from a prepopulated out directory, presumably because then the closure compiler handles the ordering?

Activity

Hide
Herwig Hochleitner added a comment -

Patch hooks up the tests into the test suite.

If you want to see the provide error, delete out before compiling.

If you want to see the failing assertion from the original test, compile a second time.

Show
Herwig Hochleitner added a comment - Patch hooks up the tests into the test suite. If you want to see the provide error, delete out before compiling. If you want to see the failing assertion from the original test, compile a second time.
Herwig Hochleitner made changes -
Field Original Value New Value
Attachment 0001-CLJS-407-Hook-up-cljs.import-test-to-the-test-runner.patch [ 11608 ]
Hide
David Nolen added a comment -

In order to better understand tickets it best not to complect different issues If there's a problem not addressed by the patch please mention that in another ticket (and feel to reference that one here).

I assume the patch fixes one thing, but reveals an unresolved issue, am I correct?

Show
David Nolen added a comment - In order to better understand tickets it best not to complect different issues If there's a problem not addressed by the patch please mention that in another ticket (and feel to reference that one here). I assume the patch fixes one thing, but reveals an unresolved issue, am I correct?
Hide
Herwig Hochleitner added a comment -

You're right, the ordering problem could be a different ticket. Sorry for that.

To clarify, there are three issues at play:

1) Part of the testsuite not executed (fixed by the patch)
2) The part that is now executed fails, didn't have time this morning to find out the original intention of Michał, can probably do tomorrow, if he doesn't pick it up.
3) The compilate is different depending on whether the .js file is cached in out/. This might require some deeper investigation.

The reason I decided to roll it into one ticket is that only addressing one of 2) and 3) leaves the test suite failing. I almost wanted to reopen CLJS-312, but decided against it, since it's water under the bridge.

Show
Herwig Hochleitner added a comment - You're right, the ordering problem could be a different ticket. Sorry for that. To clarify, there are three issues at play: 1) Part of the testsuite not executed (fixed by the patch) 2) The part that is now executed fails, didn't have time this morning to find out the original intention of Michał, can probably do tomorrow, if he doesn't pick it up. 3) The compilate is different depending on whether the .js file is cached in out/. This might require some deeper investigation. The reason I decided to roll it into one ticket is that only addressing one of 2) and 3) leaves the test suite failing. I almost wanted to reopen CLJS-312, but decided against it, since it's water under the bridge.
Hide
David Nolen added a comment -

I fixed some dependency ordering issue in another patch, would be nice to know if the ordering issues are addressed. What part of this ticket is still relevant? This is why tickets that only address one thing at a time are nice

Show
David Nolen added a comment - I fixed some dependency ordering issue in another patch, would be nice to know if the ordering issues are addressed. What part of this ticket is still relevant? This is why tickets that only address one thing at a time are nice

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated: