Skip to content

feature/smoke tests #2337

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

Merged
merged 77 commits into from
Mar 9, 2023
Merged

feature/smoke tests #2337

merged 77 commits into from
Mar 9, 2023

Conversation

baywet
Copy link
Member

@baywet baywet commented Feb 24, 2023

fixes #2020

@baywet baywet added the generator Issues or improvements relater to generation capabilities. label Feb 24, 2023
@baywet baywet self-assigned this Feb 24, 2023
@baywet baywet force-pushed the feature/smoke-tests branch from dae78e6 to 9f65643 Compare February 28, 2023 16:33
@andreaTP
Copy link
Contributor

andreaTP commented Mar 2, 2023

@baywet let me share a proposal for this PR 🙂

Would it be possible to split it into multiple PRs?
A first one only with the infrastructure needed to perform the tests, and one per language afterward to actually use the infrastructure to perform the IT tests in CI.

I would really like to properly review the infrastructure work, but it's getting a bit "noisy".
Also, by doing it with iterations we can separate the resolution of the issues that we will be found in the process.
wdyt?

@baywet
Copy link
Member Author

baywet commented Mar 2, 2023

@andreaTP Thanks for the suggestion. I'm going to implement most languages myself as part of this PR (this is core work for the GA, I don't think it'd be fair to "delegate" it to the community). The work is still early, I'm exploring things at this point and each test requires a commit to be pushed (as opposed to running things locally). I'm still expecting large changes and splitting things up at this stage would introduce a lot of coordination overhead.

As for the noise, you can ignore the PR as long as it's in draft, and only pay attention to it when it switches to ready for review.

@baywet baywet force-pushed the feature/smoke-tests branch 3 times, most recently from cdabe2f to 5f37727 Compare March 8, 2023 15:36
@baywet baywet marked this pull request as ready for review March 8, 2023 20:40
@baywet
Copy link
Member Author

baywet commented Mar 8, 2023

@andreaTP, let me know what you think.
You can see the results here

@baywet baywet requested a review from andreaTP March 8, 2023 20:40
baywet added 5 commits March 9, 2023 10:12
@baywet baywet force-pushed the feature/smoke-tests branch from 0d76d4f to ddf418e Compare March 9, 2023 15:17
@baywet baywet enabled auto-merge (squash) March 9, 2023 15:17
Copy link
Contributor

@andrueastman andrueastman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really great to bring in and setup.

Just a couple of suggestions. Otherwise, looks good to me.

@baywet baywet requested review from andrueastman and removed request for andreaTP March 9, 2023 16:13
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@baywet baywet merged commit 212ca70 into main Mar 9, 2023
@baywet baywet deleted the feature/smoke-tests branch March 9, 2023 16:32
@andreaTP
Copy link
Contributor

andreaTP commented Mar 9, 2023

Haven't had time to review, but the output seems really great! 👍 well bone @baywet !!!

@andreaTP
Copy link
Contributor

andreaTP commented Mar 9, 2023

@baywet checking this a little bit more through, awesome job!
A couple of things for the future:
Do you think we can expand and set up a MockServer so that the Integration tests will actually need to successfully perform a few calls? Or do you expect this step to be defined by each specific language?

The setup looks a bit tricky(as expected ...), do we have any "easy way" to run one of those tests for development purposes? Would a top-level Makefile or a few separate scripts help?

Based on the previous, would running into a pre-configured Docker image(with all of the tools already installed) help in terms of the reproducibility of the setup?

@baywet
Copy link
Member Author

baywet commented Mar 9, 2023

a mock server would be an awesome step forward! I think that if we want to get there we first need to address a lot of the issues that were discovered. Then we'd need to tweak the projects a bit so they make calls, or even generate the calls to make?

As per the docker image, are you thinking of adding all the language dependencies onto a single image definition or about something else?

@andreaTP
Copy link
Contributor

andreaTP commented Mar 9, 2023

are you thinking of adding all the language dependencies onto a single image definition

Not a fully formed idea unfortunately, I'm only trying to understand how the tests can be easily run during the dev cycle locally (as opposed to "in CI").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator Issues or improvements relater to generation capabilities.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kiota Coverage Test Suite
4 participants