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

Fixed panic when body type changed from array to object #357

Merged

Conversation

derbylock
Copy link
Contributor

@derbylock derbylock commented Aug 4, 2023

Fixed panic in RequestPropertyTypeChangedCheck

Example of panic:

runtime error: invalid memory address or nil pointer dereference
goroutine 1 [running]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x65
main.(*App).Execute.func1()
...
	/usr/local/go/src/runtime/panic.go:884 +0x212
github.com/tufin/oasdiff/checker.RequestPropertyTypeChangedCheck.func1({0xc0008bd366?, 0x7fab3586d108?}, {0x0?, 0x4eb385?}, 0xc0008bd370?, 0xc0008bd370?)
	/home/user/go/pkg/mod/github.com/tufin/[email protected]/checker/check-request-property-type-changed.go:52 +0x40
github.com/tufin/oasdiff/checker.processModifiedPropertiesDiff({0xc0008bd366, 0x6}, {0x0, 0x0}, 0xc0006d5760, 0x203000?, 0xc000cac958)
	/home/user/go/pkg/mod/github.com/tufin/[email protected]/checker/checks-utils.go:61 +0x7c
github.com/tufin/oasdiff/checker.processModifiedPropertiesDiff({0x0?, 0x21adb60?}, {0x0?, 0x4?}, 0xc0006d5600, 0x158e709?, 0xc000cac958)

Addtionally, fixed small typos in messages

Copy link
Contributor

@blva blva left a comment

Choose a reason for hiding this comment

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

LGTM (you may need to update breaking changes file again)

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2023

Codecov Report

Merging #357 (bf4da33) into main (30738ef) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #357      +/-   ##
==========================================
- Coverage   80.93%   80.92%   -0.01%     
==========================================
  Files         194      194              
  Lines       10536    10531       -5     
==========================================
- Hits         8527     8522       -5     
  Misses       1683     1683              
  Partials      326      326              
Flag Coverage Δ
unittests 80.92% <100.00%> (-0.01%) ⬇️

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

Files Changed Coverage Δ
checker/check-request-property-type-changed.go 94.93% <100.00%> (+0.19%) ⬆️
diff/schema_diff.go 79.19% <100.00%> (-1.07%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@derbylock
Copy link
Contributor Author

derbylock commented Aug 4, 2023

LGTM (you may need to update breaking changes file again)

I rerun ./make and it doesn't change BREAKING-CHANGES-EXAMPLES.md

Could you help me, please?

@derbylock
Copy link
Contributor Author

Looks like the order of lines in BREAKING-CHANGES-EXAMPLES.md is different on my machine and on github build nodes. I don't know what to do. I cant apply patch from build's log but it seems strange to solve such problems this way.

@reuvenharrison reuvenharrison merged commit 0098f73 into Tufin:main Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants