escalus icon indicating copy to clipboard operation
escalus copied to clipboard

added API to manually set resource

Open bartekgorny opened this issue 8 years ago • 4 comments

(needed to test case-sensitive scenarios)

bartekgorny avatar Dec 06 '17 12:12 bartekgorny

@bartekgorny Could you link some usage example of the new API?

Apart from the line comment about lists vs binaries above, there's already a way to set the resource, but it's not possible with the escalus:story/3 API. See BOSH tests for an example which uses escalus_connection:start/1 directly. The usage example would tell which API is better.

In general, I think we should avoid API bloat - we already have a lot of problems with people adding new API functions due to lack of knowledge that something is possible (like this very PR ;) ) and the broader the API, the bigger this problem will be.

erszcz avatar Jan 04 '18 15:01 erszcz

I can't link because the need for it was in MT, plus they just said it is not needed (at least for now). The usage was:

affiliation_is_case_insensitive(Config) ->
     escalus:story(Config, [{alice, 1}, {bob, "ReSurSik"}, {kate, 1}], fun(Alice, Bob, Kate) ->

There is a way to set resource without using escalus:story, there is even a way to set resource without using escalus, but we have it for a reason.

bartekgorny avatar Jan 09 '18 10:01 bartekgorny

I'm not against extending the API in general, but I'd like not to extend it by adding quirks and hacks for special cases - rather by exposing generic interfaces. To give examples:

  • https://github.com/esl/escalus/pull/162 describes the story of a hack which ultimately lead to some confusion and time consuming troubleshooting. The API was never documented appropriately.

  • In case of resource specs to escalus:story/3 (and similar) the topic was mentioned a couple of times already (informally in the office), but the path forward has never been decided. The idea is to enable passing a complete user spec to the story, which would allow for the following (or similar) syntax:

    alice(C) ->
        [ %% Alice specific opts
        ] ++ user(C).
    
    bob(C) ->
        [ %% Bob specific opts
        ] ++ user(C).
    
    kate(C) -> user(C).
    
    user(_Config) ->
        %% We don't really have to use Config, but experience shows it's quite often desirable.
        [ %% common opts for the suite
         {username, ...},
         {server, ...}
        ].
    
    C = Config,
    escalus:story(Config, [{alice(C), 1}, {bob(C), 1}, {kate(C), 1}], fun(Alice, Bob, Kate) ->
        ...
    end).
    

    OR

    alice(C) ->
        [ {resources, [<<"res1">>, <<"res2">>]}
        ] ++ user(C).
    
    escalus:story(Config, [alice(C), {bob(C), 1}, {kate(C), 1}], fun(Alice, Bob, Kate) ->
        ...
    end).
    
    

    That would even allow for deciding how resources should be generated for each user. It might make sense to use maps instead of proplists.

    It would also allow for removing users from test.config - where people add new users for new suites / new functionality, but seldom if ever reuse existing users - which ultimately decreases locality and poses problems when forking / merging tests.

    I'd be grateful for opinions on this idea.

erszcz avatar Jan 09 '18 13:01 erszcz

@erszcz 👍

Huge +1 to "exposing generic interfaces". If we'd manage to remove user specs from config files, and pass all of them explicitly instead, that would be awesome. The "we can't test certain things using story/3" scenario hit me hard many times because of its implicit behaviour (and no sane way to make it explicit!).

arkgil avatar Jan 09 '18 13:01 arkgil

Old PR, closing

arcusfelis avatar Jul 30 '24 14:07 arcusfelis