search

beyond-all-reason / teiserver

Middleware server for online gaming
https://www.beyondallreason.info/
MIT License
47 stars 47 forks source link

FixChange fake data accounts to generate a list. #283

Closed AdamChlupacek closed 2 months ago

AdamChlupacek commented 3 months ago

Fake data could not be loaded since accounts did not generated into a list.

jauggy commented 3 months ago

I was able to get Teifion's fake data script to run before. What does this PR actually do? How to test?

AdamChlupacek commented 3 months ago

If you run the fake data script which is now on master, it will fail at generating accounts. There was a breaking change introduced here, this just fixes the issue converting the ParallelStream back into a List, so that List.flatten() can actually be called and we get a list of accounts to be inserted to the DB.

jauggy commented 3 months ago

Ah I see. When I used the fake data script that was before @L-e-x-o-n made that merge.

L-e-x-o-n commented 3 months ago

Good find and fix.

I checked other places where ParallelStream is used again, this should be applied there as well. This is my mistake, I did some tests, but with unit tests being broken and parallel being used in currently mostly unused parts of Teiserver, I missed it.

Would you be interested in adding |> Enum.to_list to those other places (6 ParallelStream usages, excluding this one) as part of this PR or should I fix it in another PR?

AdamChlupacek commented 3 months ago

@L-e-x-o-n sure I can add it to the PR over the weekend.

AdamChlupacek commented 3 months ago

@L-e-x-o-n I went through the usecases and in most cases there is Enum.count already called, so it did not needed to have anything added. I only identified one spot where it needed to be added.

L-e-x-o-n commented 3 months ago

@L-e-x-o-n I went through the usecases and in most cases there is Enum.count already called, so it did not needed to have anything added. I only identified one spot where it needed to be added.

Nice, thanks.

jauggy commented 2 months ago

@StanczakDominik @Beherith I've tested and this ticket works and is important to merge. Basically if new contributors follow the setup instructions for teiserver, they will eventually need to create test data. Without this merged, they won't be able to create test data and therefore cannot complete the setup instructions.

The local setup instructions have this command for adding fake data:

mix teiserver.fakedata

that will fail on master branch currently.

Beherith commented 2 months ago

Thx!