8000 feat: Add Client for Workorders service Test plans and Test plan template APIs by ammarhusain-EMR · Pull Request #137 · ni/nisystemlink-clients-python · GitHub
[go: up one dir, main page]
Skip to content

feat: Add Client for Workorders service Test plans and Test plan template APIs #137

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 77 commits into from
May 28, 2025
Merged

feat: Add Client for Workorders service Test plans and Test plan template APIs #137

merged 77 commits into from
May 28, 2025

Conversation

ammarhusain-EMR
Copy link
Contributor

What does this Pull Request accomplish?

Added Create, Delete, and Query Test plan template APIs in Workorder service.

Why should this Pull Request be merged?

This test plan template client will improve user interactivity with those Workorder Service endpoints.

What testing has been done?

Automated integration tests are included.
API Link: Swagger-link

Ammar Husain Mian Fazulul and others added 30 commits May 16, 2025 14:55
added boiler plate for test plan client
525c770
users/ammar/feat/testplan
ebfc90f
add testplans api client and models
f09ab69
update request model name
0ebc4a5
test setup
95053ad
add create test
5d96899
lint
8eca2b3
fix test
e9dfe5d
test try 1
39630d8
test try 1
703d060
draft implementation
881a44d
moved test plan to integration folder
db0d53a
Merge branch 'users/ammar/feat/testplan' of https://github.com/aravin…
d52edd6 
…dhan-ni/nisystemlink-clients-python into users/ammar/feat/test_plan_template_api
lint
5e2699e
add tests
a08df7d
Merge branch 'users/ammar/feat/testplan' of https://github.com/aravin…
535ffd1 
…dhan-ni/nisystemlink-clients-python into users/arvin/feat/testplan/testplans-api
added rest of test cases
6100544
added documentation
293d185
updated documentation
10c8d8b
updated nit changes
0298403
added comments in init files
0b8fa56
resolved comments
99d7d9e
added documentation for test plan template
d93bbac
update test
a10a6b6
update lint
1b4b88c
add query projection
3a63524
fix lint
1977508
Merge branch 'users/ammar/feat/testplan' of https://github.com/aravin…
94054e9 
…dhan-ni/nisystemlink-clients-python into users/ammar/feat/test_plan_template_api
add query projection
bae388d
Merge branch 'users/ammar/feat/testplan' of https://github.com/aravin…
dd9cd31 
…dhan-ni/nisystemlink-clients-python into users/arvin/feat/testplan/testplans-api
Aravindhan Palanisamy added 2 commits May 23, 2025 22:47
@ammarhusain-EMR ammarhusain-EMR requested a review from rbell517 May 23, 2025 17:33
Aravindhan Palanisamy added 2 commits May 23, 2025 23:06
@aravindhan-ni
Copy link
Contributor

@rbell517, need you help

We've reviewed the code on our end and everything looks good. I also ran the tests locally against the test environment, and they all passed successfully.

However, the pipeline is failing due to a 405 error in one of the examples. It seems like the endpoint might not be available in the pipeline environment. Given that, I believe the integration test should fail in such cases. Am I missing anything?

rbell517
rbell517 reviewed May 23, 2025
View reviewed changes
@rbell517
Copy link
Collaborator
rbell517 commented May 23, 2025 •
edited
Loading

@rbell517, need you help

We've reviewed the code on our end and everything looks good. I also ran the tests locally against the test environment, and they all passed successfully.

However, the pipeline is failing due to a 405 error in one of the examples. It seems like the endpoint might not be available in the pipeline environment. Given that, I believe the integration test should fail in such cases. Am I missing anything?

@ammarhusain-EMR @aravindhan-ni
Funny situation actually. pytest is trying to run your example file as a test because it starts with test_. Examples are not intended to be run by the pipeline as tests.

You will need to modify the pytest setup in pyproject.toml to tell pytest to find tests in just the tests directory. Example toml.

[tool.pytest.ini_options]
...
testpaths = ["tests"]

@rbell517
Copy link
Collaborator

Another important note on the integration tests -- If you are creating a pull request from a fork you don't have permission to use the repo's main-test API key secret, so it will actually skip all the integration tests. You would need to make a PR in your fork and setup your own API key secret to use the github action to actually run the integration tests. I've been meaning to track down a fix to this but I haven't gotten to it.

Aravindhan Palanisamy and others added 2 commits May 24, 2025 11:17
update run test py setup
0fd415b
@aravindhan-ni @rbell517
Update tests test name
cd55dd3 
Co-authored-by: Richard Bell <418101+rbell517@users.noreply.github.com>
@ammarhusain-EMR ammarhusain-EMR requested a review from rbell517 May 26, 2025 04:54
@aravindhan-ni aravindhan-ni force-pushed the users/ammar/feat/test_plan_template_api branch from 5e41415 to f34f0fb Compare May 26, 2025 11:44
Aravindhan Palanisamy and others added 3 commits May 26, 2025 19:11
@aravindhan-ni
Copy link
Contributor
aravindhan-ni commented May 26, 2025 •
edited
Loading

Another important note on the integration tests -- If you are creating a pull request from a fork you don't have permission to use the repo's main-test API key secret, so it will actually skip all the integration tests. You would need to make a PR in your fork and setup your own API key secret to use the github action to actually run the integration tests. I've been meaning to track down a fix to this but I haven't gotten to it.

I had set up the same pipeline in forked repo. The tests are passed
image

rbell517
rbell517 approved these changes May 27, 2025
View reviewed changes
@rbell517 rbell517 enabled auto-merge (squash) May 27, 2025 16:59
auto-merge was automatically disabled May 28, 2025 07:21

Head branch was pushed to by a user without write access

@MuthuRajan-S MuthuRajan-S merged commit 953686c into ni:master May 28, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darrenbiel darrenbiel darrenbiel left review comments

@aravindhan-ni aravindhan-ni aravindhan-ni left review comments

@rbell517 rbell517 rbell517 approved these changes

@spanglerco spanglerco Awaiting requested review from spanglerco spanglerco is a code owner

@cameronwaterman cameronwaterman Awaiting requested review from cameronwaterman cameronwaterman is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ammarhusain-EMR @aravindhan-ni @rbell517 @darrenbiel @MuthuRajan-S
0