Skip to content

[Statistics] Update of README.md and test plan.#6463

Merged
driusan merged 3 commits into
aces:23.0-releasefrom
nicolasbrossard:stats_readme_and_test_plan
May 6, 2020
Merged

[Statistics] Update of README.md and test plan.#6463
driusan merged 3 commits into
aces:23.0-releasefrom
nicolasbrossard:stats_readme_and_test_plan

Conversation

@nicolasbrossard

Copy link
Copy Markdown
Contributor

Brief summary of changes

Update of README.md and TestPlan.md for the statistics module: addition of documentation and test cases related to the registration of widgets on the dashboard module.

Testing instructions (if applicable)

N/A

Link(s) to related issue(s)

@nicolasbrossard nicolasbrossard added 23.0.0-testing Category: Documentation PR or issue that aims to improve the documentation Testing PR contains test plan or automated test code (or config files for Travis) labels Apr 30, 2020
Comment thread modules/statistics/test/TestPlan.md Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the difference between "Automate Test on Travis_CI" and "Automate Test"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed ' on Travis_CI'.

@driusan driusan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it'll get confusing that the automated tests referred to in the statistics test plan are in the dashboard test.. should we move those too? Or at least explain which tests test it inside the []?

@nicolasbrossard

nicolasbrossard commented May 1, 2020

Copy link
Copy Markdown
Contributor Author

@driusan Yeah, I guess it makes more sense to move those tests from the dashboard to the stats module. I'll also remove the redundant ...on Travis CI in the [] sections. I'll do the same thing for my (similar) user account PR.

@driusan

driusan commented May 1, 2020

Copy link
Copy Markdown
Collaborator

@nicolasbrossard It might still be a good idea to reference which test it is that tests feature X in the test plan, so that anyone who wants can look it up.

@ridz1208

ridz1208 commented May 1, 2020

Copy link
Copy Markdown
Collaborator

@sruthymathew123 @nicolasbrossard

this PR seems to overlap with #6462 can you decide which one should go and close the other one please

@nicolasbrossard

Copy link
Copy Markdown
Contributor Author

@ridz1208 My PR was kept.

Comment thread modules/dashboard/test/TestPlan.md Outdated
Comment thread modules/dashboard/test/TestPlan.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer an order of magnitude on the number of candidates/sessions/instruments than a specific study. Except maybe raisinbread.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next commit will mention a reasonable display time for raisinbread.

Comment thread modules/dashboard/test/TestPlan.md Outdated
Comment thread modules/statistics/test/TestPlan.md Outdated
@driusan

driusan commented May 5, 2020

Copy link
Copy Markdown
Collaborator

@nicolasbrossard this has a conflict that needs to be resolved

Comment thread modules/dashboard/test/TestPlan.md Outdated
[Manual Test]
13. Check performance on a large dataset (like IBIS) to make sure the dashboard page displays within a reasonable
amount of time.
13. Check performance on the raisin bread dataset: make sure the dashboard page displays within, say, 5 seconds.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
13. Check performance on the raisin bread dataset: make sure the dashboard page displays within, say, 5 seconds.
13. Check performance on the raisin bread dataset: make sure the dashboard page displays within 5 seconds.

Or maybe ' within 5 seconds or less'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

@nicolasbrossard

Copy link
Copy Markdown
Contributor Author

@driusan There do not seem to be tests for the mri violation widget in the dashboard module, so I have nothing to move. I can issue another PR with tests that cover this. The new PR will also have to fix a bug in the way the widget is created in the mri violations module.

@driusan driusan merged commit 0562b4e into aces:23.0-release May 6, 2020
@ridz1208 ridz1208 added this to the 23.0.0 milestone May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Category: Documentation PR or issue that aims to improve the documentation Testing PR contains test plan or automated test code (or config files for Travis)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[statistics] Outdated test plan and README

4 participants