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

Adjust image diff threshold for testMayaLights #55

Merged
merged 5 commits into from
Feb 2, 2024

Conversation

debloip-adsk
Copy link
Collaborator

No description provided.

@debloip-adsk debloip-adsk self-assigned this Feb 2, 2024
Copy link
Collaborator

@lilike-adsk lilike-adsk left a comment

Choose a reason for hiding this comment

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

is this for MacOS specifically? I'm tweaking the threshold for another test case,

@debloip-adsk
Copy link
Collaborator Author

debloip-adsk commented Feb 2, 2024

Yes for macOS specifically, it's causing failures in preflights e.g. for #54, I've checked the output images and it does match the expected result so it looks like just a threshold issue

@lilike-adsk
Copy link
Collaborator

Yes for macOS specifically, it's causing failures in preflights e.g. for #54, I've checked the output images and it does match the expected result so it looks like just a threshold issue

If that, could we disable the test on MacOS first? I found if we use a pretty large threshold, we might miss the real failures. Meanwhile, we need to investigate why MacOS produced different images than Win/Linux for most test cases.

IMAGE_DIFF_FAIL_THRESHOLD = 0.01
IMAGE_DIFF_FAIL_PERCENT = 0.2
@property
def imageDiffFailThreshold():
Copy link
Collaborator

Choose a reason for hiding this comment

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

great, miss "self" as parameter?

Copy link
Collaborator Author

@debloip-adsk debloip-adsk Feb 2, 2024

Choose a reason for hiding this comment

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

Yes definitely, though somehow it ran fine locally without it. Thanks for catching it

@lilike-adsk lilike-adsk self-requested a review February 2, 2024 18:15
@lilike-adsk lilike-adsk added ready-for-merge Development process is finished, PR is ready for merge test labels Feb 2, 2024
@lilike-adsk lilike-adsk merged commit cdb2c23 into dev Feb 2, 2024
10 checks passed
@lilike-adsk lilike-adsk deleted the debloip/adjust-image-diff-threshold-maya-lights branch February 2, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants