Chore(test): Actor test suite #356

bakbakbakbakbak · 2023-02-17T23:03:41Z

This PR introduces tests for Actors up to Sheet level. It is not complete as I've been unable to figure all the tests out, but the test coverage should be quite high with this.

I'm putting this as a draft PR for a few days while I continue linting give a night or two of good rest to simmer.

anthonyronda · 2023-02-28T21:46:34Z

Started on this one, requesting backup because this is in large part a refactor of recent code by @wyrmisis and of course his opinion is valued :)

anthonyronda

29/36 files reviewed

I would still like to get my eyes on all of them but for now I have a few comments/questions for you to look at

anthonyronda · 2023-02-28T21:29:51Z

src/e2e/testUtils.ts


 export const closeDialogs = async () => {
  openDialogs()?.forEach(async (o) => {
    await o.close();
  });
 };

+export const closeSheets = async () => {
+  openWindows("sheet").forEach(async (w) => w.close());
+  await delay(220);


out of curiosity was 120 not enough on your machine? (as used for the dialog closing input delay constant at the top)

My machine must have had a bad day or something, I just tried changing it to the standard and it works. So I'll put this as the standard.

src/e2e/testUtils.ts

wyrmisis

I'm going to be completely honest with you: as long as these tests pass, I'm good. There is a lot here and I don't know if I have the brain space to give them the thorough reading necessary.

What's here is a good baseline, IMO. We can fix the tests as we fix bugs related to said tests.

(commenting instead of approving since I haven't been able to read all the things)

wyrmisis · 2023-02-24T02:11:00Z

src/e2e/testUtils.ts


 export const closeDialogs = async () => {
  openDialogs()?.forEach(async (o) => {
    await o.close();
  });
 };

+export const closeSheets = async () => {
+  openWindows("sheet").forEach(async (w) => w.close());
+  await delay(220);


Should we be returning this await?

I added it there (and changed it based on @anthonyronda's comment to waitForInput()) so to not having to add it everywhere we're waiting for the sheets to close. But I can change that if that is preferrable?

wyrmisis · 2023-03-20T14:33:27Z

src/module/actor/__tests__/sheet-actor.test.ts

I'm conflicted on if we should even have sheet tests if we plan on doing new sheets in the future.

UFT doesn't have to inherit them, the OSE project can inherit them. As long as you don't mind having them here for now, I can help separate the imports at the testing entrypoint later

wyrmisis · 2023-03-20T14:42:20Z

src/e2e/testUtils.ts

-  game.messages.size > 0
-    ? game.messages.documentClass.deleteDocuments([], { deleteAll: true })
+export const trashChat = (): Promise<any> =>
+  game.messages?.size > 0


Since we're using a question mark here, maybe we ought to check for if game.messages?.size is truthy, instead of checking for if it's greater than 0.

Makes sense, I think I put the question mark there as I got warnings that it may be empty. But since we are running tests with Foundry initiated, I think we can just do a straight check here.

bakbakbakbakbak · 2023-03-20T18:14:39Z

I'm going to be completely honest with you: as long as these tests pass, I'm good. There is a lot here and I don't know if I have the brain space to give them the thorough reading necessary.

What's here is a good baseline, IMO. We can fix the tests as we fix bugs related to said tests.

(commenting instead of approving since I haven't been able to read all the things)

All the tests won't pass, so it is a question if we should merge the test suite and work off the failing tests. When I run with all the fixes I've put together I get 1069/1129 tests passing right now though.

wyrmisis · 2023-03-20T19:06:50Z

I'm going to be completely honest with you: as long as these tests pass, I'm good. There is a lot here and I don't know if I have the brain space to give them the thorough reading necessary.
What's here is a good baseline, IMO. We can fix the tests as we fix bugs related to said tests.
(commenting instead of approving since I haven't been able to read all the things)

All the tests won't pass, so it is a question if we should merge the test suite and work off the failing tests. When I run with all the fixes I've put together I get 1069/1129 tests passing right now though.

Any chance you could list out what all is failing, and what all will pass when whatever PRs you have out there go in? No biggie if not, I know that's a fair bit of book-keeping.

bakbakbakbakbak · 2023-03-20T20:45:41Z

Note: Lots of the failing tests are due to large quantity of HD rolling tests.

I have a branch based from this being merged that contains the fixes in fix(item): Dragging a container onto itself pops it from inventory sheet #359.

Failing Tests

This branch as-is: 861/1128
With #359: 883/1128 tests

With HD fixes (#372 ): 1044/1129 tests, failing tests:

character data model:
- #spellList
- init()
monster data model:
- isNew()
- init()
actor entity:
- update()
- rollAppearing()
- rollDamage()
actor sheet:
- _toggleContainedItems
- _toggleItemSummary
- _displayItemInChat
- _removeItemFromActor
- _onSpellChange
- _resetSpells
- _rollAbility
- _rollAttack
- _createItem
- _onConfigureActor
actor sheet: drag & drop
- _onDragStart -> _onDrop for containers
character sheet:
- generateScores()
Actor entity tweaks
- defaultOptions
- title()
Container data model
- totalWeight
item entity
create()
prepareDerivedData
spendSpell
pushManualTag
show
party sheet
- _onDropFolder
party helpers
- update
treasure helpers
- drawTreasure

With #372 & #359: 1067/1129
fixes these tests:

actor sheet: drag & drop
- _onDragStart -> _onDrop for containers

I have commits to fix:

party sheet (fix(party sheet): Fixes dropping folders of players onto party sheet #374)
- _onDropFolder
treasure helpers (fix: treasure made asynchronous #373)
- drawTreasure
Actor entity tweaks (fix(dialog): Added class for Entity Tweaks windows for 1.8.x #351 )
- defaultOptions
- title()

Remaining failing tests

character data model:
- #spellList
- init()
monster data model:
- isNew()
- init()
actor entity:
- update()
- rollAppearing()
- rollDamage()
actor sheet:
- _toggleContainedItems
- _toggleItemSummary
- _displayItemInChat
- _removeItemFromActor
- _onSpellChange
- _resetSpells
- _rollAbility
- _rollAttack
- _createItem
- `_onConfigureActor
character sheet:
- generateScores()
Container data model
- totalWeight
item entity
create()
prepareDerivedData
spendSpell
pushManualTag
show
party helpers
- update

anthonyronda · 2023-03-27T23:43:26Z

@bakbakbakbakbak thanks for listing the failing tests. If you'd like to, please share your WIPs for the ones on a local branch :)

anthonyronda

LGTM

I will be honest, I did not scour each and every test (I'm particularly worried about false positives, which are harder to pick out)

I did look over them generally. If there are some mistakes in here, it's not the end of the world or even going to affect people's games too much :)

Thanks for such a gigantic contribution 🎉

anthonyronda · 2023-03-27T23:47:06Z

src/e2e/testUtils.ts

-  game.messages.size > 0
-    ? game.messages.documentClass.deleteDocuments([], { deleteAll: true })
-    : null;
+export const trashChat = (): any => {


Don't know if this passes linting for the current ESLint config. That said, we may have to revisit discussion of the FVTT Types project anyway...

anthonyronda · 2023-03-28T00:03:42Z

I'm hoping to re-spark discussion on a path forward for accepting fixes on the remaining failing tests before merging. Let us know how we can help forge a path forward!

My own view is that it's much more helpful for us to collaborate on fixes if we have the failing tests in 1.8.x. I don't want to merge 1.8.x into main, however, until all tests are green

bakbakbakbakbak · 2023-03-28T05:02:52Z

@bakbakbakbakbak thanks for listing the failing tests. If you'd like to, please share your WIPs for the ones on a local branch :)

I've submitted the PRs #372 #373 #374 now (and references them in the comment above)!

bakbakbakbakbak marked this pull request as ready for review February 18, 2023 07:14

bakbakbakbakbak requested a review from anthonyronda February 18, 2023 07:14

bakbakbakbakbak mentioned this pull request Feb 27, 2023

fix(item): Dragging a container onto itself pops it from inventory sheet #359

Merged

anthonyronda requested a review from wyrmisis February 28, 2023 21:45

anthonyronda mentioned this pull request Feb 28, 2023

fix(dialog): Added class for Entity Tweaks windows for 1.8.x #351

Merged

anthonyronda reviewed Mar 3, 2023

View reviewed changes

bakbakbakbakbak added 10 commits March 5, 2023 08:32

chore(utils): testUtils tweaks

405c269

chore(test): Actor testing up to sheet

30bea74

chore(lint): Test linting & remova assert(false)

d5f0acf

chore(structure): Better naming of test suite

637bb13

chore(structure): Fixed remaining test names

28709ea

fix(test): revert closeSheets to standard delay

048031a

fix(test): renamed cleanUpActorsKey

a4a5b37

chore(lint): spacing and linting to dice tests

caedd8b

chore(lint): sheet linewidth linting

17e9c93

fix: defined type for key in cleanUpActorsByKey

14a41df

wyrmisis reviewed Mar 20, 2023

View reviewed changes

fix: wyrmisis comments

1056765

bakbakbakbakbak force-pushed the chore/actor_test_suite branch from 3ab54c6 to 1056765 Compare March 20, 2023 17:46

anthonyronda approved these changes Mar 27, 2023

View reviewed changes

This was referenced Mar 28, 2023

fix(actor): monster hd rolls #372

Draft

fix: treasure made asynchronous #373

Closed

fix(party sheet): Fixes dropping folders of players onto party sheet #374

Closed

fix: actor entity tweaks #375

Closed

anthonyronda merged commit a0280f6 into vttred:1.8.x Mar 28, 2023

anthonyronda mentioned this pull request Mar 29, 2023

Container contains itself, then disappears #357

Closed

1 task

bakbakbakbakbak deleted the chore/actor_test_suite branch April 9, 2023 07:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore(test): Actor test suite #356

Chore(test): Actor test suite #356

bakbakbakbakbak commented Feb 17, 2023

anthonyronda commented Feb 28, 2023

anthonyronda left a comment

anthonyronda Feb 28, 2023

bakbakbakbakbak Mar 3, 2023

wyrmisis left a comment

wyrmisis Feb 24, 2023

bakbakbakbakbak Mar 20, 2023

wyrmisis Mar 20, 2023

anthonyronda Mar 27, 2023

wyrmisis Mar 20, 2023

bakbakbakbakbak Mar 20, 2023

bakbakbakbakbak commented Mar 20, 2023

wyrmisis commented Mar 20, 2023

bakbakbakbakbak commented Mar 20, 2023 •

edited

Loading

anthonyronda commented Mar 27, 2023

anthonyronda left a comment

anthonyronda Mar 27, 2023

anthonyronda commented Mar 28, 2023

bakbakbakbakbak commented Mar 28, 2023 •

edited

Loading

Chore(test): Actor test suite #356

Chore(test): Actor test suite #356

Conversation

bakbakbakbakbak commented Feb 17, 2023

anthonyronda commented Feb 28, 2023

anthonyronda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wyrmisis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bakbakbakbakbak commented Mar 20, 2023

wyrmisis commented Mar 20, 2023

bakbakbakbakbak commented Mar 20, 2023 • edited Loading

Failing Tests

Remaining failing tests

anthonyronda commented Mar 27, 2023

anthonyronda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthonyronda commented Mar 28, 2023

bakbakbakbakbak commented Mar 28, 2023 • edited Loading

bakbakbakbakbak commented Mar 20, 2023 •

edited

Loading

bakbakbakbakbak commented Mar 28, 2023 •

edited

Loading