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

change propeller compiler test #4823

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Feb 2, 2024

Update how the propeller compile test works.

  • Removed all yaml files.
  • Commented out the branch directory test since walking through it, it didn't actually do anything. Leaving it there as a comment for now.
  • Some of the tests were literally testing for what encoding: provide canonical output format golang/protobuf#1121 advises against testing for. That is, we were serializing proto objects to yaml and checking that they string matched the saved yaml files. Removed these tests (which is also why the yaml files were deleted.)
  • However, since it's possible to unmarshal from json, we can use the existing json files to reconstruct a proto object and do a proto equals.
  • The last check at the bottom of the loop however is not a proto file, it's the Flyte workflow crd, so convert to a new json file, and then make the tester compare strings where multiple spaces have all been converted to one space.

Signed-off-by: Yee Hing Tong <[email protected]>
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. housekeeping Issues that help maintain flyte and keep it tech-debt free labels Feb 2, 2024
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (buf-experiment@00ab086). Click here to learn what that means.

Additional details and impacted files
@@                Coverage Diff                @@
##             buf-experiment    #4823   +/-   ##
=================================================
  Coverage                  ?   59.76%           
=================================================
  Files                     ?      462           
  Lines                     ?    37273           
  Branches                  ?        0           
=================================================
  Hits                      ?    22278           
  Misses                    ?    13323           
  Partials                  ?     1672           
Flag Coverage Δ
unittests 59.76% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 2, 2024
@wild-endeavor wild-endeavor merged commit bbdd6a6 into buf-experiment Feb 2, 2024
42 of 48 checks passed
@wild-endeavor wild-endeavor deleted the buf-experiment-propeller-test branch February 2, 2024 16:55
eapolinario added a commit that referenced this pull request Feb 6, 2024
* use buf

Signed-off-by: Yee Hing Tong <[email protected]>

* Remove java and cpp.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add es and connect-es to buf.gen.yaml and generate files

Signed-off-by: Eduardo Apolinario <[email protected]>

* Generated go code.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add generated python code.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Uncomment openapiv2 definitions.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove go and python swagger-generated code

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove swagger-related code from makefile and scripts

Signed-off-by: Eduardo Apolinario <[email protected]>

* Stop generating code for cpp and java

Signed-off-by: Eduardo Apolinario <[email protected]>

* Stop go code generation in script

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove docs generated code

Signed-off-by: Eduardo Apolinario <[email protected]>

* Do not generate openapiv2 asset

Signed-off-by: Eduardo Apolinario <[email protected]>

* Lint

Signed-off-by: Eduardo Apolinario <[email protected]>

* Update go.mod

Signed-off-by: Eduardo Apolinario <[email protected]>

* Drop os from flyteadmin service

Signed-off-by: Eduardo Apolinario <[email protected]>

* make go-tidy

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix test

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix datacatalog

Signed-off-by: Eduardo Apolinario <[email protected]>

* Lint flyteadmin, flyteplugins, and flytepropeller.

Signed-off-by: Eduardo Apolinario <[email protected]>

* fix up datacatalog tests

Signed-off-by: Yee Hing Tong <[email protected]>

* one more test fix

Signed-off-by: Yee Hing Tong <[email protected]>

* fix tests

Signed-off-by: Yee Hing Tong <[email protected]>

* plugin test

Signed-off-by: Yee Hing Tong <[email protected]>

* more test failure, the spacing seems to be different on gh

Signed-off-by: Yee Hing Tong <[email protected]>

* starting admin

Signed-off-by: Yee Hing Tong <[email protected]>

* Fix flyteadmin tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* change propeller compiler test (#4823)

Signed-off-by: Yee Hing Tong <[email protected]>

* Add regex to fix tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Pin versions of typescript buf plugins

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add utility AssertEqualWithSanitizedRegex and fix flyteadmin tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix integration test

Signed-off-by: Eduardo Apolinario <[email protected]>

* Lint

Signed-off-by: Eduardo Apolinario <[email protected]>

* More linting

Signed-off-by: Eduardo Apolinario <[email protected]>

* Lint again

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add protoc-gen-openapiv2 as a dependence in flyteidl

Signed-off-by: Eduardo Apolinario <[email protected]>

* Serve swagger file using golang's embed.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix datacatalog tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix a few more flyteadmin tests

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Yee Hing Tong <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Issues that help maintain flyte and keep it tech-debt free lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants