(Add tests to) document how multiple inceptors work #3564
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The current test contains a lot of details that are worth being tested separately and also made explicit. This is what is coming with the next changes.
The more explicit the test, the better we can 1) describe what it does and use that for the docs to make them understandable 2) have a better explicit description of the API (in case we want to modify it) and 3) have a regression test when changing.
… they were added.
… things they care about.
only the last one in the chain is returned.
The previous change to also catch in the helper function was not needed for any of the tests above, so remove it and put it in the describe-block below where it is needed. Just trying to write the specific code needed, not more. Not sure about the impact for failing tests, I am glad about input. But the main intention here is to have minimal code, explicit tests and not too much generic code which might break itself eventually.
I just extracted the test descriptions from the new/modified tests. And I also learned that I should improve the test descriptions, the tests are just in a GIVEN-WHEN-THEN structure, so I can also use those terms better. Will do this next.
…ADME back into the tests. After I had updated the README using the test descriptions I realized how the test descriptions can be improved, so I did this "moving" the learnings back into the code.
mbargiel
pushed a commit
to mbargiel/axios
that referenced
this pull request
Jan 27, 2022
* Add a group for the multiple-interceptors tests. The current test contains a lot of details that are worth being tested separately and also made explicit. This is what is coming with the next changes. * Fix indentation. * Make explicit that the test only tests the interception of the fulfilled part. * Indent. * Make the test just test a tiny bit, as described. The more explicit the test, the better we can 1) describe what it does and use that for the docs to make them understandable 2) have a better explicit description of the API (in case we want to modify it) and 3) have a regression test when changing. * Add a test that explicitly shows that the order of interceptors is as they were added. * Pull out reusable code, to make the tests more explicitly stating the things they care about. * Write a test showing that the responses are not merged or anything, only the last one in the chain is returned. * Make visible that the interceptors are a chain, one receives its predecessor's data. * Test what happens when the interceptor throws. * Refactor, add another group of tests and pull helper function there. The previous change to also catch in the helper function was not needed for any of the tests above, so remove it and put it in the describe-block below where it is needed. Just trying to write the specific code needed, not more. Not sure about the impact for failing tests, I am glad about input. But the main intention here is to have minimal code, explicit tests and not too much generic code which might break itself eventually. * Documenting that the following reject-interceptor gets called. * Documenting how the interceptor chain handles caught rejections. * Document "multiple interceptors". I just extracted the test descriptions from the new/modified tests. And I also learned that I should improve the test descriptions, the tests are just in a GIVEN-WHEN-THEN structure, so I can also use those terms better. Will do this next. * Transfer the better readable descriptions as I changed them in the README back into the tests. After I had updated the README using the test descriptions I realized how the test descriptions can be improved, so I did this "moving" the learnings back into the code. * Be a bit more explicit, it was not clear when reading, imho. * Add new lines to be kinda consistent with the rest of the file. Co-authored-by: Jay <jasonsaayman@gmail.com>
This was referenced Feb 22, 2022
This was referenced Mar 9, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Document Multiple Inceptors
TL;DR
This PR adds a new sub chapter "Multiple Inceptors" in the docs in the README. In order to understand and prove what goes in the docs I extended the tests.
Details
I used axios in a project and while I was reading the docs to learn how multiple interceptors work, I needed to dig all the way into the tests and into the source code to finally understand it fully.
This led me to improve the tests to document how axios handles multiple interceptors. I refactored the tests and added some, that imho make it more explicit how it works.
In order to update the docs, which was my initial goal, I pulled out the descriptions from the tests and put them in the README to improve the docs.
My main intent here was to enhance the docs so that people who also look for how multiple inceptors work will find it faster in the docs. I hope this helps.