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

Qunit2 support? #155

Open
binodpanta opened this issue Aug 4, 2016 · 20 comments
Open

Qunit2 support? #155

binodpanta opened this issue Aug 4, 2016 · 20 comments

Comments

@binodpanta
Copy link
Collaborator

Looks like there is no version of Funcunit yet that support QUnit2? Is there a plan to release FuncUnit for qunit2, if so, what is the timeline on it?

@binodpanta
Copy link
Collaborator Author

Let alone QUnit2, I think it also does not work with qunit 1.23.1, I tried to upgrade to that version, I got this error in this function,

"Uncaught Error: Called stop() outside of a test context"

/[email protected]#browser/adapters/qunit/
define('funcunit/browser/adapters/qunit', function (require, exports, module) {
module.exports = function (QUnit) {
return {
pauseTest: function () {
QUnit.stop();
},
resumeTest: function () {
QUnit.start();
},
assertOK: function (assertion, message) {
QUnit.ok(assertion, message);
},
equiv: function (expected, actual) {
return QUnit.equiv(expected, actual);
}
};
};
});

@matthewp
Copy link
Contributor

matthewp commented Aug 9, 2016

Timeline is when it's done. Please send a PR if you have it working.

Not sure about your other problem with 1.23.1, can you provide a breaking example?

@binodpanta
Copy link
Collaborator Author

hi there, so it turns out that as of QUnit 1.17, the error occurs only if I use F.open() inside the QUnit.moduleStart() callback

I attached a set of examples to demonstrate this

this was the reason some of our tests were failing

Question is: are we doing something out of model or is FuncUnit? Resolving this will help us move to 1.23.1 I think which will be great as we prepare to do 2.0

To run, open attached zip and run testpage-1.14.html. It works
Then run testpage-1.17.html. It shows this failure
I am sure the same happens with 1.23.html too but I did not go that far even at this time

funcunitexample.zip

@matthewp
Copy link
Contributor

Thanks for the example!

@binodpanta
Copy link
Collaborator Author

hello there, please let me know if I can help in any way here. I am prepping to submit the sendKeys in another pull request so I am sort of setup to run the tests locally and all that, so if you have any ideas I can try, I am do that. Apparently the use of QUnit.start/stop are the problems, but I am not sure what to replace it with since there are no globals anymore!

@matthewp
Copy link
Contributor

No globals is ok, you pass QUnit to FuncUnit using F.attach(QUnit) anyways.

Here's the code that detects if the thing you pass into .attach is QUnit: https://github.com/bitovi/funcunit/blob/master/browser/adapters/adapters.js#L84-L86

I don't know what has changed with QUnit2 to tell you if/how that needs to change. But it needs to correctly detect that the QUnit object is the QUnit object, if that makes sense.

From there you need to update the qunit adapter (or create a new one specially for qunit2): https://github.com/bitovi/funcunit/blob/master/browser/adapters/qunit.js

Without knowing all what has changed I can't guide too much here unfortunately. One problem could be if QUnit2 no longer has stuff like QUnit.equal but instead only has the test-local assert.equal versions as adapters are not really set up with that in mind. Not sure what should be done in that case, might need some sort of test-local version of QUnit.attach.

@binodpanta
Copy link
Collaborator Author

binodpanta commented Aug 24, 2016

ok so I find that if I replace

QUnit.moduleStart(function() {  F.open("./state_verification_testpage.html"); } )

with the following, I get rid of this problem,

  QUnit.module("FuncUnit state verification tests", {

  beforeEach: function(assert) {
    F.open("./state_verification_testpage.html");
}, 
    afterEach: function(assert) {
        F.wait(1, function() { F.win.close();});
    }
});

According to the qunit 2 upgrade guide, QUnit.moduleStart is still supported if called directly as a function
http://qunitjs.com/upgrade-guide-2.x/#removed-and-modified-qunit-methods-and-properties

So whatever way QUnit is checking that QUnit.start and stop are being called outside of test content, that check fails in the first scenario but passes in the second. Interesting.

However, if I use the above, the test fails in a different way where click is not working.

@binodpanta
Copy link
Collaborator Author

ok with Qunit 1.23.1 I have fully verified that the use of F.open/wait etc in QUnit.moduleStart/moduleDone fails like above but the same functions when used in the beforeEach and afterEach callbacks of QUnit do work

    // This works with QUnit 1.23.1
    QUnit.module("FuncUnit state verification tests", {

        beforeEach: function(assert) {
            F.open("./state_verification_testpage.html");
        }, 
        afterEach: function(assert) {
            F.wait(1, function() { F.win.close();});
        }

    });

    // This does not work with QUnit 1.23.1

    QUnit.moduleStart(function() { 
        //F.open("./state_verification_testpage.html");
    });
    QUnit.moduleDone(function() { 
        //F.wait(1, function() { F.win.close();});
    });

Next step for me is to repeat with QUnit 2.x and see what happens.

This may be an acceptable workaround but it will require us to change thousands of files perhaps in our test bed, so a bit annoying in the least

@binodpanta
Copy link
Collaborator Author

Unfortunately the solution may not be that simple after all from my limited knowledge of this. Unless QUnit somehow allows this type of usage.

Here is what I have concluded thus far

  1. Funcunit uses QUnit.start() and QUnit.stop() to resumeTest and pauseTest as part of F.open, F.close etc.
  2. Qunit as of >1.17 no longer allows the use of QUnit.stop()/start() outside test content. And code running in moduleStart() is apparently considered outside test content now
  3. I looked at the possibility of using the new assert.async() which is replacing the use of QUnit.start() and QUnit.stop() in QUnit2.x. But that api also is not usable from within FuncUnit's pauseTest and resumeTest because these calls
  4. If I just call F.open in beforeEach I am all fine, nothing breaks with no changes, but stuff starts to fail again if I use F.close in afterEach or anywhere for that matter. So as soon as I use F.close, subsequent F.open calls fail with this message Uncaught TypeError: Cannot read property 'replace' of undefined@ 115 ms Source: http://localhost:61193/test/funcunit/funcunit.js:2645

More specifically, if I try to use assert.async in FuncUnit's QUnit adapter like this,

define('funcunit/browser/adapters/qunit', function (require, exports, module) {
    module.exports = function (QUnit) {
        var done;
        return {
            pauseTest: function () {
                done = QUnit.assert.async();
            },
            resumeTest: function () {
                done();
            },
            assertOK: function (assertion, message) {
                QUnit.ok(assertion, message);
            },
            equiv: function (expected, actual) {
                return QUnit.equiv(expected, actual);
            }
        };
    };
});

I get this error

    TypeError: Cannot read property 'semaphore' of undefined
        at Object.async (http://localhost:61193/test/funcunit/qunit.js:1393:7)
        at Object.pauseTest (http://localhost:61193/test/funcunit/funcunit.js:2555:34)
        at Function.FuncUnit.add (http://localhost:61193/test/funcunit/funcunit.js:3196:27)
        at Function.open (http://localhost:61193/test/funcunit/funcunit.js:2660:22)
        at Object.<anonymous>

So I am not sure that the next logical step is here. Whether it is to have QUnit somehow allow this type of usage, or for us to stop using F.open/close in moduleStart/stop. The latter can be done if there is no other way.

@shayaneumar
Copy link

shayaneumar commented Apr 4, 2017

I have made following changes in these files make this work with QUnit 2

  • Passing local asset to FuncUnit
    funcunit\browser\core.js
FuncUnit = function( selector, frame ) {
		// if you pass true as context, this will avoid doing a synchronous query
		var frame,
			forceSync, 
			isSyncOnly = false;

	   var callerFirstArgument = FuncUnit.caller.arguments[0];

	   if(callerFirstArgument && callerFirstArgument.test){
				FuncUnit.assert = callerFirstArgument;
	   }

2- Use local assert in place of global assert
funcunit\browser\adapters\qunit.js

if(window.QUnit) {
  var done,
   assert;

  FuncUnit.unit = {
  pauseTest:function(){
   assert = FuncUnit.assert;
   done = assert.async();
  },
  resumeTest: function(){
   done();
  },
  assertOK: function(assertion, message){
   assert.ok(assertion, message);
  },
  equiv: function(expected, actual){
   return QUnit.equiv(expected, actual);
  }
}

@binodpanta
Copy link
Collaborator Author

Thanks @shayaneumar, do you plan to submit these? Does this allow fully updating to a new QUnit version 1.14 or higher?

@shayaneumar
Copy link

shayaneumar commented Apr 18, 2017

@binodpanta I have tested it only with qunit v2.3.2, changes suggested here are also not backward compatible. I am not very confident with the code change above I was thinking anyone from the team can help me with this, I want o upgrade my tests to qunit2 asap. I will try to raise a PR for this if I get some time.

@kdillon
Copy link
Collaborator

kdillon commented Apr 18, 2017

If it isn't backward compatible, we can always release it as a major-version update.

@justinbmeyer
Copy link
Member

I've not really been following this. Is it possible to test which version of QUnit is present and do the right thing?

@shayaneumar
Copy link

@binodpanta here is the PR to support that stealjs/steal-qunit#26.
I had to make changes in steel-qunit as well to support that shayaneumar/steal-qunit@6724346

@kjdonahue
Copy link

This update does not seem to resolve the initial issue raised by @binodpanta where calling F.open() from a callback passed to QUnit.moduleStart() throws. In 1.23.1 the Uncaught Error: Called stop() outside of a test context message is present. In >=2.0, the following message is present:
TypeError: Cannot read property 'semaphore' of undefined at Object.async (http://localhost:61193/test/funcunit/qunit.js:1393:7) at Object.pauseTest (http://localhost:61193/test/funcunit/funcunit.js:2555:34) at Function.FuncUnit.add (http://localhost:61193/test/funcunit/funcunit.js:3196:27) at Function.open (http://localhost:61193/test/funcunit/funcunit.js:2660:22) at Object.<anonymous>

@binodpanta
Copy link
Collaborator Author

indeed this will be a major blocker for us

@binodpanta
Copy link
Collaborator Author

Just want to raise this point (I have asked Kevin to discuss this with the core group at some point)

I just wanted to make this point that this problem is demonstrable if you take FuncUnit out of the picture and do this ( feel free to use this content when you discuss with the FuncUnit team, if needed )

<head><link rel="stylesheet" href="https://code.jquery.com/qunit/qunit-1.23.1.css"></head>
  
<script src="https://code.jquery.com/qunit/qunit-1.23.1.js"></script>
<body>
<div id="qunit"></div>
  <div id="qunit-fixture"></div>
  
<script >
QUnit.config.autostart = false;

QUnit.moduleStart(function() {
    QUnit.stop();< ============================ this line fails with this error as of qunit1.17 or higher
});

QUnit.test("hello", function(assert) {
    assert.ok(1==1,'failed');
});

setTimeout(function() {QUnit.start()}, 3000);
</script>
</body>

So the analysis here is:

  1. QUnit does not allow Qunit.stop to be used in moduleStart/stop since it is not considered Test content according to it
  2. FuncUnit’s F.open/close when using Qunit adapter, uses QUnit.start/stop
  3. Hence this problem also reflects in F.open/close

Question now is , which of these is approaches is the best fix?

  1. Should Qunit make a change to allow this?
  2. Should FuncUnit use methods other than QUnit.start/stop within F.open/close?
  3. Tests should avoid use F.open/close in moduleStart/Stop

Puzzle for me is :
It appears that @shayaneumar 's fix no longer uses QUnit.start/stop so I wonder why this still does not work?

@m-mujica
Copy link
Contributor

m-mujica commented May 21, 2017

hey @binodpanta, Manuel here, I work at Bitovi and I'd like to help solve this issue.

I'm kinda confused about what's exactly the problem here.

  1. Why are you using QUnit.moduleStart? I would not use moduleStart/moduleDone instead of beforeEach/afterEach; the former are useful hooks if you need to collect test metadata or implement your own reporter. The latter are meant to setup code specific to the suite of tests you're running.

That said, if you use beforeEach/afterEach do you run into any issues?

QUnit.module( "module A", {
  before: function() {
    // prepare something once for all tests
  },
  beforeEach: function() {
    // prepare something before each test
  },
  afterEach: function() {
    // clean up after each test
  },
  after: function() {
    // clean up once after all tests are done
  }
});

if you have a breaking example that would be useful, too. I looked into the zip file you shared and rewriting your test to:

F.attach(QUnit);

QUnit.module("testing qunit 1.17", {
  before: function() {
    F.open("./anotherpage.html");
  };
});

QUnit.test("atest", function(assert) {
  F("body").visible("assertion needed");
  F.wait(1, function(){F.win.close();});
});

made it pass!

@m-mujica
Copy link
Contributor

if you are concerned about changing a bunch of tests, you could look into a codemod to make it easier. We recently migrated all stealjs tests to Qunit 2.x stealjs/steal#1154 and with qunit-migrate it was not bad at all.

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

No branches or pull requests

8 participants