esl / escalus

An XMPP client library in Erlang for conveniently testing XMPP servers
Apache License 2.0
129 stars 76 forks source link

include test case name in fresh user's name #179

Closed ludwikbukowski closed 4 years ago

ludwikbukowski commented 6 years ago

the users will be named with case name suffix, like:

for better debugging purposes. It assumes that in int_per_testcase/2 escalus:init_per_testcase/2 was run

michalwski commented 6 years ago

I wonder if this would work with long test case names, we do have them.

ludwikbukowski commented 6 years ago

maybe I can add truncating for the case name

michalwski commented 6 years ago

@ludwikbukowski any progress on this? are you going to truncate the names if they are too long? Also could you create a PR to esl/MongooseIM with these changes?

fenek commented 6 years ago

I believe the strongest limit on username length we have is the SQL schema and even there it's 250 characters. And I don't think we have test case name that is longer than ~100 characters. It shouldn't break the tests then but I'm wondering if there is any risk of leaving more unpredictable leftovers if the usernames will have even less chance of collision. I guess not, but... :)

ludwikbukowski commented 6 years ago

I wrote this "feature" mostly for debugging purposes. Imagine you run 10 different tests SUITES and at the end you see that there is one unregistered user alice123.43. It would take time to figure out which test case created this user. Or, for some reason in some test (lets say groupchat tests), all of the sudden, you receive message from bob33.1 (because there is some shared room between tests or whatever) Those are just examples from my head. And I don't think there will be characters limit hit. However, I'm more aware that escalus:init_per_testcase won't be called every time before test and there will be users like alice_unnamed_32.4 created - but then it's worth to investigate why it's not called

fenek commented 6 years ago

Sounds good to me. Is there a respective MIM PR for this change?

ludwikbukowski commented 6 years ago

I created one: https://github.com/esl/MongooseIM/pull/1939

erszcz commented 4 years ago

MongooseIM run https://github.com/esl/MongooseIM/pull/1939 has shown that this naming scheme doesn't cause issues. I'm merging this one, since it seems handy.