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

Relax expectation so encoder test passes on ARM #183

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

chapulina
Copy link
Contributor

🦟 Bug fix

Fixes #180

Summary

The difference on that expectation for all ARM builds (arm64 and armhf) were exactly 1.86, so I think relaxing the tolerance to 2.0 should fix them.

I haven't looked closely enough at #125, but quickly looking at the test this seems like a reasonable solution to me.

CC @peci1

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

@github-actions github-actions bot added 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Mar 9, 2021
@peci1
Copy link
Contributor

peci1 commented Mar 9, 2021

Thanks for the fix. As I commented at #180 , if you agree with setting the limit to 2.0, I'm okay with it ;)

@peci1
Copy link
Contributor

peci1 commented Mar 9, 2021

I just wonder this wasn't discovered by nightly builds... or are they done just for x86?

@codecov
Copy link

codecov bot commented Mar 9, 2021

Codecov Report

Merging #183 (e01daef) into ign-common3 (38c0d0c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-common3     #183   +/-   ##
============================================
  Coverage        75.39%   75.39%           
============================================
  Files               72       72           
  Lines            10249    10249           
============================================
  Hits              7727     7727           
  Misses            2522     2522           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38c0d0c...e01daef. Read the comment docs.

@chapulina
Copy link
Contributor Author

As I commented at #180 , if you agree with setting the limit to 2.0, I'm okay with it ;)

Nice, I hadn't even seen that!

are they done just for x86?

Yup, we've only been releasing nightlies for amd64

@chapulina chapulina mentioned this pull request Mar 9, 2021
7 tasks
@chapulina chapulina merged commit 0bddf0f into ign-common3 Mar 9, 2021
@chapulina chapulina deleted the chapulina/3/encoder_arm branch March 9, 2021 01:43
@chapulina chapulina added the tests Broken or missing tests / testing infra label Mar 9, 2021
chapulina added a commit that referenced this pull request Mar 19, 2021
* 🎈 3.11.0 (#178)

Signed-off-by: Louise Poubel <[email protected]>

* Relax expectation so encoder test passes on ARM (#183)

Signed-off-by: Louise Poubel <[email protected]>

* Revert "Associate library materials effect with meshes (#151)" (#182)

This reverts commit 89b1157.

Signed-off-by: Louise Poubel <[email protected]>

* 🎈 3.11.1 (#184)

Signed-off-by: Louise Poubel <[email protected]>

* Master branch updates (#179)

Part of gazebo-tooling/release-tools#416

Signed-off-by: Louise Poubel <[email protected]>

* Remove use of _SOURCE and _BINARY dirs in tests (#158)

Test cleanup:

Merges test/util.hh and test_config.h into a single file (fixes include path issues)
Adds fixtures for retrieving test data from test/data folder
Moves testing utilities into ignition::common::testing

Signed-off-by: Michael Carroll <[email protected]>
Co-authored-by: Jose Luis Rivero <[email protected]>

Co-authored-by: Michael Carroll <[email protected]>
Co-authored-by: Jose Luis Rivero <[email protected]>
This was referenced Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome tests Broken or missing tests / testing infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

INTEGRATION_video_encoder fails on arm64, armhf debbuilds
2 participants