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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve documentation: make message queue cleaning clearer #122

Merged
merged 6 commits into from Jun 27, 2023

Conversation

ctmbl
Copy link
Contributor

@ctmbl ctmbl commented Jun 23, 2023

Should fix #116

According to me, this is the least that should be done to help the new user deal with message queue, the must would be to perform it automatically.

However I'm still not sure about a few changes:

  • have I updated everything that should be?
  • is there other queues that should be emptied? send_queue, error_queue?
  • in Message queue not empty at test start up聽#116 we didn't speak of the tuto without pytest but it should suffer the same issue right?
  • I tried to improve the paragraph introducing fixture to talk about tear down/cleaning, which is what we're using here
  • I added a troubleshooting case, the more explicit I could, to help future users that could encounter the issue I encountered

I separated my changes in different commits to help you walking through the reviewing process.

Don't hesitate to reject or discuss any changes I propose 馃槈

@ctmbl
Copy link
Contributor Author

ctmbl commented Jun 23, 2023

@Sergeileduc

@Sergeileduc
Copy link
Collaborator

hmmm

I think the part in test_ping :

    assert dpytest.verify().message().contains().content("Ping:")
    await dpytest.empty_queue() # empty the global message queue as test teardown

can be confusing, for new users.
It may lead to think "oh I should always put that line in my tests"

We definitly need to warn users that this problem can occur, and provide that information, but I think it would be better if it's after the "basic use case" without it.

I think we can keep the examples simple and straightforward, as they are, and right after, write somethint like "one problem that could happen is that the sent_queue is shared between the tests, so in order to not mess between your tests (verify() pops one message from the queue, so in general, you won't need to do anything, but if you have any issues, you can explicitly call empty_queue, as shown in the next example (and later, in the conftests.py)

something like that, I think

@ctmbl
Copy link
Contributor Author

ctmbl commented Jun 26, 2023

@Sergeileduc

I really like your idea! I'll change it that way

@ctmbl
Copy link
Contributor Author

ctmbl commented Jun 26, 2023

Could you just confirm that the syntax I used in db1d9cd won't be a problem?
I've never right Sphinx (I think it's sphinx right?) documentation and can't test it locally I think

@Sergeileduc
Copy link
Collaborator

Sergeileduc commented Jun 27, 2023

@ctmbl yeah, it's OK, '-' will make a bulleted list

I'll add the makefile and make.bat for the doc, so everyone can build and test (either on Windows or Unix)

And the tasks for invoke, so you'll just need to type inv doc to build and open the doc in your browser
(well, if you pip install Sphinx and invoke, of course, but I think they are in the dev-requirements.txt)

@Sergeileduc Sergeileduc merged commit 5f64700 into CraftSpider:master Jun 27, 2023
5 checks passed
@ctmbl ctmbl deleted the empty-queue-documentation branch June 27, 2023 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Message queue not empty at test start up
2 participants