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

Make a short topology fixture alias #2144

Closed
389-ds-bot opened this issue Sep 13, 2020 · 6 comments
Closed

Make a short topology fixture alias #2144

389-ds-bot opened this issue Sep 13, 2020 · 6 comments
Labels
closed: fixed Migration flag - Issue
Milestone

Comments

@389-ds-bot
Copy link

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/49085


There is one more idea/proposal for the create_test.py improving and the way we invoke topology objects.

Instead of writing tests like this:

from lib389.topologies import topology_m2

def test_something(t):
    """Test case"""

    topology_m2.ms["master1"].stop()
    topology_m2.ms["master1"].start()

We can write like this:

from lib389.topologies import topology_m2 as t

# We can use it in our code, optionally
m1 = t.ms["master1"]

def test_something(t):
    """Test case"""

    t.ms["master1"].stop()
    m1.start()

For that, I can refactor create_test.py a bit, so it will make all initial actions for you. What do you think?

@389-ds-bot 389-ds-bot added the closed: fixed Migration flag - Issue label Sep 13, 2020
@389-ds-bot 389-ds-bot added this to the lib389 1.0.4 milestone Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-01-11 04:35:50

I don't like the short variable names like "t" as it gives no context to what is happening. I'm guilty sometimes of doing things like this, but I don't think we should encourage it widely.

A compromise would be "import topology_m2 as topo" or something like that, which is shorter but at least still indicative or a partial word.

Ultimately, is this really a big problem for us? What will this solve for us? How does it help us?

@389-ds-bot
Copy link
Author

Comment from spichugi (@droideck) at 2017-01-11 11:06:04

Replying to [comment:1 Firstyear]:

I don't like the short variable names like "t" as it gives no context to what is happening. I'm guilty sometimes of doing things like this, but I don't think we should encourage it widely.

A compromise would be "import topology_m2 as topo" or something like that, which is shorter but at least still indicative or a partial word.
I agree with you, I don't like and don't use such variables too.
Topology is a bit another thing. It has its "main" part of the name i.e. "master", "consumer etc.
So in my opinion,

t.ms["master1"]

is still very readable. Though I do fully agree to the "'''topo'''" too, it is still better than "'''topology_m1h1c1'''".

Ultimately, is this really a big problem for us? What will this solve for us? How does it help us?
It is not a big at all, I've set "minor" flag to it. :) We discussed it with Viktor recently and both agreed that it will make our code cleaner and easier to "type".
And to refactor this, I will spend no more than a few tens of minutes.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-01-11 12:03:04

Yep, I'm convinced now. Let's go with your plan. The reason I say topo not "t", is search/replace on "t" may not easily work, but at least topo is somewhat unique in a search.

@389-ds-bot
Copy link
Author

@389-ds-bot
Copy link
Author

Comment from spichugi (@droideck) at 2017-01-16 22:11:53

To ssh://git.fedorahosted.org/git/389/ds.git
c969a82..9fcfb6c master -> master
commit 9fcfb6c
Author: Simon Pichugin droideck@redhat.com
Date: Mon Jan 16 13:47:41 2017 +0100

@389-ds-bot
Copy link
Author

Comment from spichugi (@droideck) at 2017-02-11 22:59:35

Metadata Update from @droideck:

  • Issue assigned to droideck
  • Issue set to the milestone: lib389 1.0.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: fixed Migration flag - Issue
Projects
None yet
Development

No branches or pull requests

1 participant