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

Implement runCreateInputCheck in terms of Create #2503

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Implement runCreateInputCheck in terms of Create #2503

merged 1 commit into from
Oct 18, 2024

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Oct 18, 2024 •
edited
Loading

@iwahbe
Implement runCreateInputCheck in terms of Create
e43d020 
stack-info: PR: #2503, branch: iwahbe/stack/4
iwahbe added a commit that referenced this pull request Oct 18, 2024
@iwahbe
Implement runCreateInputCheck in terms of Create
e98e8d3 
stack-info: PR: #2503, branch: iwahbe/stack/4
This was referenced Oct 18, 2024
Merged
Merged
Merged
@iwahbe iwahbe requested review from VenelinMartinov and a team October 18, 2024 12:20
@iwahbe iwahbe changed the base branch from iwahbe/stack/3 to master October 18, 2024 12:22
@iwahbe iwahbe changed the base branch from master to iwahbe/stack/3 October 18, 2024 12:22
@iwahbe iwahbe self-assigned this Oct 18, 2024
@codecov Codecov
Copy link

codecov bot commented Oct 18, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.60%. Comparing base (563065f) to head (55b8bca).

Additional details and impacted files 
@@                Coverage Diff                 @@
##           iwahbe/stack/2    #2503      +/-   ##
==================================================
+ Coverage           62.50%   62.60%   +0.10%     
==================================================
  Files                 381      381              
  Lines               51335    51318      -17     
==================================================
+ Hits                32088    32130      +42     
+ Misses              17432    17374      -58     
+ Partials             1815     1814       -1

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

iwahbe added a commit that referenced this pull request Oct 18, 2024
@iwahbe
Implement runCreateInputCheck in terms of Create
8da7d66 
stack-info: PR: #2503, branch: iwahbe/stack/4
iwahbe added a commit that referenced this pull request Oct 18, 2024
@iwahbe
Implement runCreateInputCheck in terms of Create
c249217 
stack-info: PR: #2503, branch: iwahbe/stack/4
@iwahbe iwahbe changed the base branch from iwahbe/stack/3 to master October 18, 2024 16:29
@iwahbe iwahbe force-pushed the iwahbe/stack/4 branch 2 times, most recently from c249217 to 9f99778 Compare October 18, 2024 16:29
@iwahbe iwahbe mentioned this pull request Oct 18, 2024
Merged
@iwahbe iwahbe changed the base branch from master to iwahbe/stack/2 October 18, 2024 16:29
iwahbe added a commit that referenced this pull request Oct 18, 2024
@iwahbe
Implement runCreateInputCheck in terms of Create
dcec1ab 
stack-info: PR: #2503, branch: iwahbe/stack/4
t0yv0
t0yv0 reviewed Oct 18, 2024
View reviewed changes
pkg/internal/tests/cross-tests/create.go
@@ -103,17 +106,49 @@ func Create(
assertCtyValEqual(t, "RawConfig", tfResult.data.GetRawConfig(), puResult.data.GetRawConfig())
assertCtyValEqual(t, "RawPlan", tfResult.data.GetRawPlan(), puResult.data.GetRawPlan())
assertCtyValEqual(t, "RawState", tfResult.data.GetRawState(), puResult.data.GetRawState())

for k := range resource {
// TODO: make this recursive
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean here? How can we get to 100%?

Copy link
Member

Choose a reason for hiding this comment

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

If I remember right, we are comparing some eeky structures here coming from the TFSDK, these structures resist directly comparing for equality but this is something we need for the purposes of the tests. It is a little tedious to get right but sounds like it should be doable within an hour or two. Am I understanding right? Need a hand here perhaps?

@iwahbe iwahbe changed the base branch from iwahbe/stack/2 to master October 18, 2024 16:38
@iwahbe iwahbe changed the base branch from master to iwahbe/stack/2 October 18, 2024 16:38
t0yv0
t0yv0 approved these changes Oct 18, 2024
View reviewed changes
Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

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

Looks like my comments are out of scope for the refactor.

iwahbe added a commit that referenced this pull request Oct 18, 2024
@iwahbe
Implement runCreateInputCheck in terms of Create
55b8bca 
stack-info: PR: #2503, branch: iwahbe/stack/4
@iwahbe iwahbe changed the base branch from iwahbe/stack/2 to master October 18, 2024 17:16
@iwahbe iwahbe changed the base branch from master to iwahbe/stack/2 October 18, 2024 17:16
iwahbe added a commit that referenced this pull request Oct 18, 2024
@iwahbe
Implement runCreateInputCheck in terms of Create
df19353 
stack-info: PR: #2503, branch: iwahbe/stack/4
Base automatically changed from iwahbe/stack/2 to master October 18, 2024 19:39
@iwahbe iwahbe merged commit 1dd0380 into master Oct 18, 2024
17 checks passed
@iwahbe iwahbe deleted the iwahbe/stack/4 branch October 18, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t0yv0 t0yv0 t0yv0 approved these changes

@VenelinMartinov VenelinMartinov Awaiting requested review from VenelinMartinov

Assignees

@iwahbe iwahbe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iwahbe @t0yv0