Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Some tests are not referenced in a test suite #300

Closed
wants to merge 1 commit into from

3 participants

@divdavem
Collaborator

This pull request adds the following tests in the corresponding suite and fixes them (so that the build is successful):

  • test.aria.templates.issue142.HtmlStyleTemplateTestCase
  • test.aria.templates.issue279.ButtonSpacingTestCase
@divdavem divdavem referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@divdavem divdavem closed this in 9956682
@piuccio

I've integrated the changes with commit 9956682 but we should have a task that detects orphan tests.
I'm thinking about a grunt task...

@piuccio piuccio reopened this
@susant123 susant123 referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@jakub-g
Collaborator

I'm wondering what would be the best solution for this.

We can't rely on file names because the majority of the test cases doesn't have any Test part in their name.

We could list all .js files in test dir, and filter only those which contain the magic words to be the test case (because some 30% are tpl scripts and other util JS files used by tests etc. so we need to exclude them).

The best way to check for this would be to look at $extends because each test case/suite extends, directly or not, the base test case/suite file.

The problems are with indirect inheritance.

I've quickly analyzed the contents of the folder and we have test cases/suites that extend from:

$extends : 'aria.jsunit.TestCase',
$extends : 'aria.jsunit.RobotTestCase',
$extends : 'aria.jsunit.TemplateTestCase',
$extends : "aria.jsunit.ModuleCtrlTestCase",
$extends : "aria.jsunit.WidgetTestCase",

$extends : "test.aria.widgets.container.issue80.shared.BindableSizeTestCase",
$extends : "test.aria.widgets.form.multiselect.issue312.Common",

and all test suites extend from

$extends : "aria.jsunit.TestSuite",

Regarding test cases, the last two are families of very specialized per-issue test cases inheriting from the base test cases and differing only in widgets under tests.

So if we renamed the last file say to Issue312TestCase then we could analyze the contents of the files looking for match for $extends : *TestCase match (either with regex or AST).

Then we need to obtain all the tests added by this.addTests( ) in the suites and compare the two arrays.

@jakub-g
Collaborator

I'm in favor of wildcard match, because we have e.g. aria.jsunit.MultiSelectTemplateTestCase in CC, and we may potentially add new in the future, so a bulletproof solution will be preferred.

BTW we should add some marker to BindableSizeTestCase and issue312.Common saying that those classes are abstract test cases ($abstract : true ?) and exclude them.

@jakub-g
Collaborator

We can also go the other way and rename all test files to end with Test / TestCase - this will at least not need to read 300 files from disk to do the check...

@piuccio

The best would be to remove the TestSuite altogether, they are just a duplication of information.
The test runner should be able to understand which files are test cases and run them all.

@jakub-g
Collaborator

I'm closing this as this is an old specific pull request, I opened a new issue #924 for tracking the general issue (I don't like pull request to be visible as open while they were merged).

@jakub-g jakub-g closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 19, 2012
  1. @divdavem
This page is out of date. Refresh to see the latest.
View
2  test/aria/templates/TemplatesTestSuite.js
@@ -48,6 +48,8 @@ Aria.classDefinition({
this.addTests("test.aria.templates.ViewTest");
this.addTests("test.aria.templates.generatedId.IncrementalElementIdTestCase");
+ this.addTests("test.aria.templates.issue142.HtmlStyleTemplateTestCase");
+ this.addTests("test.aria.templates.issue279.ButtonSpacingTestCase");
this.addTests("test.aria.templates.statements.StatementsTestSuite");
View
26 test/aria/templates/issue142/HtmlStyleTemplateTestCase.js
@@ -5,9 +5,9 @@ Aria.classDefinition({
$constructor : function () {
this.$TemplateTestCase.constructor.call(this);
this.setTestEnv({
- template : "test.aria.templates.issue142..HtmlStyleTemplate"
+ template : "test.aria.templates.issue142.HtmlStyleTemplate"
});
-
+
},
$prototype : {
runTemplateTest : function () {
@@ -17,25 +17,25 @@ Aria.classDefinition({
scope : this,
delay : 1000
});*/
-
-
+
+
},
-
+
__checkStyle : function(){
this.__testCase1();
this.__testCase2();
this.__testCase3();
this.__testCase4();
this.__finishTest();
-
-
+
+
},
__testCase1:function(){
var domId, domStyle;
-
+
domId = this.getWidgetInstance("id1")._domId;
domStyle = aria.utils.Dom.getElementById(domId);
-
+
this.assertTrue(domStyle.style.height == "100px", "The TPL1 height is not correct " + domStyle.style.height);
this.assertTrue(domStyle.style.width == "200px", "The TPL1 width is not correct " + domStyle.style.width);
},
@@ -43,7 +43,7 @@ Aria.classDefinition({
var domId, domStyle;
domId = this.getWidgetInstance("id2")._domId;
domStyle = aria.utils.Dom.getElementById(domId);
-
+
this.assertTrue(domStyle.style.height == "150px", "The TPL2 height is not correct " + domStyle.style.height);
this.assertTrue(domStyle.style.width == "400px", "The TPL2 width is not correct " + domStyle.style.width);
},
@@ -51,16 +51,16 @@ Aria.classDefinition({
var domId, domStyle;
domId = this.getWidgetInstance("id3")._domId;
domStyle = aria.utils.Dom.getElementById(domId);
-
+
this.assertTrue(domStyle.style.height == "", "The TPL3 height is not correct " + domStyle.style.height);
this.assertTrue(domStyle.style.width == "", "The TPL3 width is not correct " + domStyle.style.width);
-
+
},
__testCase4:function(){
var domId, domStyle;
domId = this.getWidgetInstance("id4")._domId;
domStyle = aria.utils.Dom.getElementById(domId);
-
+
this.assertTrue(domStyle.style.height == "", "The TPL4 height is not correct " + domStyle.style.height);
this.assertTrue(domStyle.style.width == "", "The TPL4 width is not correct " + domStyle.style.width);
},
View
36 test/aria/templates/issue279/ButtonSpacingTemplate.tpl
@@ -1,26 +1,20 @@
{Template {
- $classpath:'test.aria.templates.issue279.ButtonSpacingTemplate',
- $wlibs : {
- html : "aria.html.HtmlLibrary"
- }
+ $classpath:'test.aria.templates.issue279.ButtonSpacingTemplate'
}}
- {macro main()}
-
-
- <div>
- {@aria:Button {
- id: "id1",
- label:"Enable/Disable Important Button",
- onclick : ""
- } /}
- {@aria:Button {
- id: "id2",
- label:"Enable/Disable Important Button",
- onclick : ""
- } /}
- </div>
-
- {/macro}
+ {macro main()}
+ <div>
+ {@aria:Button {
+ id: "id1",
+ label:"Button 1",
+ onclick : ""
+ } /}
+ {@aria:Button {
+ id: "id2",
+ label:"Button 2",
+ onclick : ""
+ } /}
+ </div>
+ {/macro}
{/Template}
View
1  test/aria/templates/issue279/ButtonSpacingTestCase.js
@@ -27,6 +27,7 @@ Aria.classDefinition({
button2Position = domUtil.calculatePosition(button2DomStyle);
space = button2Position.left - button1Position.left - button1DomStyle.offsetWidth;
+ this.assertTrue(button1Position.top == button2Position.top, "Buttons are not aligned on the same line");
this.assertTrue(space >= 1, "Button spacing between button is less than 1 px, actual:" + space);
this.assertTrue(space < 3, "Button spacing between button is more than 3 px, actual:" + space);
},
Something went wrong with that request. Please try again.