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.

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

feat: add support for TES v1.1.0 #31

Merged
merged 9 commits into from
Mar 28, 2023
Merged

feat: add support for TES v1.1.0 #31

merged 9 commits into from
Mar 28, 2023

Conversation

LakiG
Copy link
Collaborator

@LakiG LakiG commented Feb 20, 2023 •
edited
Loading

Fixes #11.

Added TES v1.1.0 pydantic model

@codecov
Copy link

codecov bot commented Feb 20, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.07 🎉

Comparison is base (9f2de35) 99.62% compared to head (5784e62) 99.69%.

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev      #31      +/-   ##
==========================================
+ Coverage   99.62%   99.69%   +0.07%     
==========================================
  Files          10       11       +1     
  Lines         527      653     +126     
==========================================
+ Hits          525      651     +126     
  Misses          2        2
Impacted Files Coverage Δ
compliance_suite/constants/constants.py 100.00% <ø> (ø)
compliance_suite/models/v1_0_0_specs.py 100.00% <ø> (ø)
compliance_suite/functions/client.py 100.00% <100.00%> (ø)
compliance_suite/job_runner.py 100.00% <100.00%> (ø)
compliance_suite/models/v1_1_0_specs.py 100.00% <100.00%> (ø)
compliance_suite/test_runner.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@LakiG LakiG requested a review from uniqueg February 20, 2023 19:49
@lbeckman314
Copy link
Contributor

This looks really good @LakiG! Any other updates needed before @uniqueg completes a review?

@LakiG
Copy link
Collaborator Author

LakiG commented Mar 1, 2023 •
edited
Loading

Thanks @lbeckman314. This PR will just have the new model and basic backward compatibility to the existing codebase.
I will add new tests (filter functionality) separately.

@lbeckman314
Copy link
Contributor

Ah, understood! Would it work for me to rebase the other tests from #29 onto this PR for future development?

@LakiG
Copy link
Collaborator Author

LakiG commented Mar 3, 2023

Yes, that would work.

uniqueg
uniqueg requested changes Mar 9, 2023
View reviewed changes
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

Cool stuff. I think the unit tests would really benefit from making use of parametrization, especially to make them more future proof when additional TES versions appear, or when generalizing the test runner for use with other specs (which likely use other versions).

compliance_suite/job_runner.py Show resolved Hide resolved
compliance_suite/test_runner.py Show resolved Hide resolved
unittests/test_job_runner.py Outdated Show resolved Hide resolved
unittests/test_job_runner.py Outdated Show resolved Hide resolved
unittests/test_test_runner.py Outdated Show resolved Hide resolved
@uniqueg
Copy link
Member

uniqueg commented Mar 9, 2023

Very sorry for my very late review, been too busy! 🙈
However, this was the last issue in my backlog, so next ones will be much faster!

@LakiG
Copy link
Collaborator Author

LakiG commented Mar 10, 2023

No problem, thank you Alex for reviewing.
I have added parameterization, its quite helpful. I also did basic cleanup/refactoring, though it can be further improved.

@LakiG LakiG requested a review from uniqueg March 10, 2023 16:38
uniqueg
uniqueg approved these changes Mar 11, 2023
View reviewed changes
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

Awesome!

However, before merging, let's first pull in @lbeckman314's test cases :)

@LakiG LakiG mentioned this pull request Mar 23, 2023
Merged
lbeckman314 and others added 2 commits March 25, 2023 21:44
@lbeckman314
Feature/tesv1.1 (#32)
2d97eaa 
* Add tests for optional filetypes in inputs and outputs

* Add test for wildcards in task outputs

* Add test for backend_parameters flag in task resources

* Add v1.0/v1.1 compatible test for resources in task creation test

* Add test for streamable flag in tesInput

* Update title and description in streamable flag test

* Add test for ignore_error flag

* Update title and description in ignore_error flag test

* Add test for CANCELING state to delete cloud resources

- Add check for CANCELING state in functions test

* Update v0.1 task creation tests

- Remove extra executors
- Change executor images from ubuntu to alpine
- Update tags with specific test names
- Change default executor commands to `echo hello`
- Update version format to fit new versioning scheme
- Remove v1.0.0 from v1.1 tests
@LakiG
refactor: simplify polling state check
8a213f7
@LakiG LakiG requested a review from uniqueg March 27, 2023 20:44
uniqueg
uniqueg approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

The PR is too big for me to efficiently review. But from what I can see it looks good to me, and we have already discussed and resolved several issues. I'd say, let's merge it, and if we stumble on any bugs or unintended behaviors, we can create issues for those.

@LakiG
Copy link
Collaborator Author

LakiG commented Mar 28, 2023

Sure. I have also reviewed it multiple times now. I think it should work.
Merging this.

@LakiG LakiG merged commit c8a4515 into dev Mar 28, 2023
@LakiG LakiG deleted the feature/tes-v1_1_0 branch March 28, 2023 07:59
This was referenced Mar 29, 2023
Closed
Closed
Closed
lbeckman314 added a commit to lbeckman314/tes-compliance-suite that referenced this pull request Apr 22, 2023
@lbeckman314
feat: add support for TES v1.1.0 (elixir-cloud-aai#31)
1e5282e 
Co-authored-by: Liam Beckman <lbeckman314@gmail.com>
lbeckman314 added a commit to lbeckman314/tes-compliance-suite that referenced this pull request Apr 28, 2023
@lbeckman314
feat: add support for TES v1.1.0 (elixir-cloud-aai#31)
e48f7f1 
Co-authored-by: Liam Beckman <lbeckman314@gmail.com>

feat: add filter functionality and tests (elixir-cloud-aai#33)

feat: add test file description in report (elixir-cloud-aai#41)

feat: add path parameters (elixir-cloud-aai#44)

feat: add test for ignore_error flag (TES v1.1.0) (elixir-cloud-aai#45)

feat: add tests for pagination options (elixir-cloud-aai#47)

feat: support query parameter transformation (elixir-cloud-aai#46)

Exit test suite with error code if any tests fail
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uniqueg uniqueg uniqueg approved these changes

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

Successfully merging this pull request may close these issues.

Add support for TES v1.1
3 participants
@LakiG @lbeckman314 @uniqueg