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

Hotfix/include graphql in 2.1.0 schema #7

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

marlon-sousa
Copy link
Contributor

In this pr, we perform a couple of things:

  1. We update the postman collection v 2.1.0 schema from draft to stable.
  2. We adequate the generated structs to comply with the stablized schema, namely including graphql, some kinds of authentication and overal small changes.
  3. We update version and authors to include colaborators.

Attention

We also have found that tests were not running, as the initial path from where collections were loaded was being prefixed with "/", which means an absolute test folder (e.e /tests). This was obviously wrong, and to further prove that we inserted random characters in the json collections. Tests were passing, which means that the collections were not even loaded.
As we removed the "/" (slash) from the initial collections path, thus using now a path relative to where cargo test is run, we got the invalid serialization errors, which means the collections were now being loaded as expected.

We performed these tests in a Linux environment, using bash. As far as we are aware, all other shells in Linux and Mac OS will consider paths prefixed with a "/" as absolute, so this needed to be fixed. Always refer to relative paths either beginning with the relative folder name directly (no slash) or using "./" to be explicit on the fact that . (a "magic" directory name which means the current directory) should be considered as the starting point to calculate sub folders.

With that fix, tests were run and failed. I didn't fix them because, frankly, I still don't understand what they are supposed to do (not saying they make no sense, only saying I could not understand what they are supposed to test). I ask you please to run cargo test and fix these tests. For reference, even the aster branch (without this pr applied) also failed, so the changes introduced here are not responsible for that. As I said, tests were passing because the collections were not even loaded. I would code these tests to fail if no ccjson files are found, to further prevent this from happening again.

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.

2 participants