Closed dhovart closed 2 years ago
It would be great if you could test locally and make sure the styles and behaviours are what you expect before merging.
Merging #327 (e3d9537) into master (b74b9a8) will decrease coverage by
0.44%
. The diff coverage is50.00%
.
@@ Coverage Diff @@
## master #327 +/- ##
==========================================
- Coverage 62.46% 62.02% -0.45%
==========================================
Files 16 16
Lines 2342 2341 -1
==========================================
- Hits 1463 1452 -11
- Misses 879 889 +10
Flag | Coverage Δ | |
---|---|---|
integration | 52.15% <50.00%> (-1.39%) |
:arrow_down: |
unittests | 43.87% <50.00%> (-0.07%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
controller/mri/mri.controller.js | 73.57% <0.00%> (-0.27%) |
:arrow_down: |
controller/user/user.controller.js | 86.66% <ø> (-0.15%) |
:arrow_down: |
controller/project/project.controller.js | 86.22% <100.00%> (-0.05%) |
:arrow_down: |
controller/atlasmakerServer/atlasmakerServer.js | 42.91% <0.00%> (-0.91%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update b74b9a8...e3d9537. Read the comment docs.
@katjaq @ntraut @r03ert0 let me know if the tests relying on the DOM seem correct to you.
@katjaq @ntraut @r03ert0 let me know if the tests relying on the DOM seem correct to you.
for me it's better like this. not sure that all the important elements are evaluated but anyway we still can add some evaluations if we find something important missing. maybe it could be good to still produce the screenshots even if we don't use them for the evaluation, just to have a simple visual way of checking if something went wrong.
@katjaq @ntraut @r03ert0 let me know if the tests relying on the DOM seem correct to you.
for me it's better like this. not sure that all the important elements are evaluated but anyway we still can add some evaluations if we find something important missing. maybe it could be good to still produce the screenshots even if we don't use them for the evaluation, just to have a simple visual way of checking if something went wrong.
there are a lot of page.waitForSelector
but the blocks have no timeout. not sure it is good because if one selector is not present the test will hang forever.
It defaults to a timeout of 30000ms. I think it's a good thing if it fails because the selector was not encountered. You can shorten the timeout of you think it's better.
Em qua., 2 de mar. de 2022 14:25, Nicolas Traut @.***> escreveu:
@katjaq https://github.com/katjaq @ntraut https://github.com/ntraut @r03ert0 https://github.com/r03ert0 let me know if the tests relying on the DOM seem correct to you.
for me it's better like this. not sure that all the important elements are evaluated but anyway we still can add some evaluations if we find something important missing. maybe it could be good to still produce the screenshots even if we don't use them for the evaluation, just to have a simple visual way of checking if something went wrong.
there are a lot of page.waitForSelector but the blocks have no timeout. not sure it is good because if one selector is not present the test will hang forever.
— Reply to this email directly, view it on GitHub https://github.com/neuroanatomy/BrainBox/pull/327#issuecomment-1056929568, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF2VIELSOJITKDSAZBLEGDU55T55ANCNFSM5O5C2V4A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you authored the thread.Message ID: @.***>
indeed, i looked at the mocha timeout which is sometimes set to 0 but the puppeteer function itself has a default timeout of 30s, sorry about that miss
for me it is ok to merge
This PR uses the Vue components I've created here : https://github.com/dhovart/nwl-components They're set to be similar in style and to behave the same as what is currently existing.
The idea is to reuse the same components for both microdraw and brainbox (and to provide building block for coming up with custom components). A config file is used to provide site-specific information and customization.
Just the settings, new project and user pages have been set to use them for now. For the project page, it requires much more effort to move everything to Vue.