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

Bumps in garden: use ign-common5 #224

Merged
merged 4 commits into from
Dec 29, 2021

Conversation

chapulina
Copy link
Contributor

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Dec 27, 2021
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Dec 27, 2021
Signed-off-by: Louise Poubel <[email protected]>
@codecov
Copy link

codecov bot commented Dec 29, 2021

Codecov Report

Merging #224 (8fe711d) into main (67af73e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #224   +/-   ##
=======================================
  Coverage   75.55%   75.55%           
=======================================
  Files          20       20           
  Lines        2757     2757           
=======================================
  Hits         2083     2083           
  Misses        674      674           

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 67af73e...8fe711d. Read the comment docs.

@scpeters scpeters changed the title Bumps in garden : ci_matching_branch/bump_garden_ign-fuel-tools8 Bumps in garden: use ign-common5 Dec 29, 2021
@scpeters
Copy link
Member

@osrf-jenkins run tests please

1 similar comment
@scpeters
Copy link
Member

@osrf-jenkins run tests please

Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Dec 29, 2021
Signed-off-by: Louise Poubel <[email protected]>
@scpeters
Copy link
Member

some console output from one of the windows test failures, not sure if it's relevant:

[ RUN      ] FuelClientTest.CachedModel
5: [Dbg] [C:\Jenkins\workspace\ign_fuel-tools-pr-win\ws\ign-fuel-tools\src\FuelClient_TEST.cc:48] Creating local model in [C:\Jenkins\workspace\ign_fuel-tools-pr-win\ws\build\ignition-fuel_tools8]
5: [Wrn] [C:\Jenkins\workspace\ign_fuel-tools-pr-win\ws\ign-fuel-tools\src\LocalCache.cc:105] Server directory does not exist [C:\Jenkins\workspace\ign_fuel-tools-pr-win\ws\build\ignition-fuel_tools8\test_cache\fuel.ignitionrobotics.org]
5: C:\Jenkins\workspace\ign_fuel-tools-pr-win\ws\ign-fuel-tools\src\FuelClient_TEST.cc(621): error: Value of: result
5:   Actual: false
5: Expected: true

@chapulina chapulina mentioned this pull request Dec 29, 2021
7 tasks
@chapulina
Copy link
Contributor Author

I opened a separate PR targeting Citadel with the test updates from 8fe711d:

@chapulina
Copy link
Contributor Author

some console output from one of the windows test failures, not sure if it's relevant:

Thanks, I was looking for the error code in the test below:

5: �[mC:\Jenkins\workspace\ign_fuel-tools-pr-win\ws\ign-fuel-tools\src\FuelClient_TEST.cc(621): error: Value of: result
5:   Actual: false
5: Expected: true
5: C:\Jenkins\workspace\ign_fuel-tools-pr-win\ws\ign-fuel-tools\src\FuelClient_TEST.cc(622): error: Expected equality of these values:
5:   ResultType::FETCH_ALREADY_EXISTS
5:     Which is: 4-byte object <05-00 00-00>
5:   result.Type()
5:     Which is: 4-byte object <07-00 00-00>

It's getting a FETCH_ERROR, I'll track where that can be coming from

@chapulina
Copy link
Contributor Author

I think it's the colon on the directory paths (#204). Someone needs dig into this and solve it for all versions. There are inconsistencies on how fuel-tools and common handle this for different versions 😕

@chapulina
Copy link
Contributor Author

Ok, so Windows has test failures on Edifice and Fortress, and the Windows check is not required on those branches. I think we should also remove the requirement for Garden, merge this, and when #106 is fixed for Edifice, the fix should be forward-ported and CI cleaned up.

What do you think, @scpeters ?

@chapulina
Copy link
Contributor Author

I removed the requirement for Windows CI on main and updated

I think this is ready!

@chapulina chapulina merged commit c47d85f into main Dec 29, 2021
@chapulina chapulina deleted the ci_matching_branch/bump_garden_ign-fuel-tools8 branch December 29, 2021 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants