Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scenario: Confusion between parameter name and function name #520

Closed
ikedas opened this issue Jan 9, 2019 · 6 comments · Fixed by #559
Closed

Scenario: Confusion between parameter name and function name #520

ikedas opened this issue Jan 9, 2019 · 6 comments · Fixed by #559
Assignees
Milestone

Comments

@ikedas
Copy link
Member

ikedas commented Jan 9, 2019

Version

6.2.38 and earlier.

Installation method

Any.

Expected behavior

By implicit inclusion feature of scenario file, "include.<action>.header is automatically added to evaluated scenarios".

Actual behavior

If a scenario corresponds to a compound list parameter, that parameter name is used as <action> above, instead of scenario function name.

Example:

Scenario file d_read.xxx corresponds to list parameter shared_doc.d_read. It tries to include header file include.shared_doc.d_read.header instead of include.d_read.header.

Additional information

  • PR to fix this bug will be submitted soon.
  • This bug has been included in very earlier version of Sympa, and it is probable that several sites use incorrect file names to enable inclusion. Correction should be announced at the time of new release.
@ikedas
Copy link
Member Author

ikedas commented Jan 10, 2019

I withdrew the PR I have submitted. Because I found that several other confusions between function name and parameter name are found in current code.

I'm planning refactoring (rearranging) code related to scenario feature. These bug will be solved with that work (in early this year).

@dverdin
Copy link
Contributor

dverdin commented Jan 10, 2019

The general rule being that the scenario name matches the parameter name, we should stick to this and rename scenarios when a parameter is renamed. For example, when d_read became shared_doc.d_read.
In that case, the scenario code is not faulty. It does exactly what it is supposed to do.

@ikedas
Copy link
Member Author

ikedas commented Jan 11, 2019

@dverdin, that rule is no longer enforced. See for example archive.web_access parameter vs. archive_web_access function, while tracking.tracking vs. tracking. From the beginning, topic_visibility function does not have corresponding list/domain parameter.

Another example of confusions between function and parameter is dump_scenario page of wwsympa. This doesn't work with scenarios for shared documents.

@dverdin
Copy link
Contributor

dverdin commented Jan 11, 2019

So how to do? shall we keep a table of pairs such as (parameter => scenario_name)? That would easily solve this problem.

@ikedas
Copy link
Member Author

ikedas commented Jan 11, 2019

I'll submit a PR later (in this month or next). Basically, it may solely use function name to specify scenario function with new(), request_auth() etc., and gives a table mapping function names to list/domain parameters.

@ikedas ikedas changed the title Inconsistent naming of files for scenario inclusion feature Scenario: Confusion between parameter name and function name Jan 12, 2019
@ikedas
Copy link
Member Author

ikedas commented Mar 3, 2019

PR above may fix this issue. I'd like to merge it in this week.

@ikedas ikedas added the ready A PR is waiting to be merged. Close to be solved label Mar 3, 2019
ikedas added a commit that referenced this issue Mar 8, 2019
@ikedas ikedas removed the ready A PR is waiting to be merged. Close to be solved label Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants