-
Notifications
You must be signed in to change notification settings - Fork 97
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
Remove JSON examples from artifacts #450
base: dev
Are you sure you want to change the base?
Conversation
Closes Green-Software-Foundation#440 Signed-off-by: Yasumasa Suenaga <[email protected]>
Signed-off-by: Yasumasa Suenaga <[email protected]>
Signed-off-by: Yasumasa Suenaga <[email protected]>
Signed-off-by: Yasumasa Suenaga <[email protected]>
This a stale pull request. Please review, update or/and close as necessary. |
This PR conflicts current |
Signed-off-by: Yasumasa Suenaga <[email protected]>
Signed-off-by: Yasumasa Suenaga <[email protected]>
Signed-off-by: Yasumasa Suenaga <[email protected]>
Signed-off-by: Yasumasa Suenaga <[email protected]>
Signed-off-by: Yasumasa Suenaga <[email protected]>
@@ -15,7 +15,7 @@ namespace CarbonAware.WepApi.IntegrationTests; | |||
[TestFixture(DataSourceType.JSON)] | |||
[TestFixture(DataSourceType.WattTime)] | |||
[TestFixture(DataSourceType.ElectricityMaps)] | |||
[TestFixture(DataSourceType.ElectricityMapsFree)] | |||
//[TestFixture(DataSourceType.ElectricityMapsFree)] // TODO: need to implement data source into IntegrationTestingBase.cs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to document better why this is being ignored for this PR please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated my Visual Studio Community 2022 to version 17.12.3 , then all tests are passed without commented-out.
This screenshot shows EMFree datasource tests in both CarbonAwareControllerTests.cs and LocationsControllerTests.cs have been skipped. I wonder why they were skipped, but it might be the issue in VS (or test facilities in dotnet
) because the issue has gone in the latest VS.
Can someone evaluate on your environment without //
? I will remove //
if it works on other environment(s).
Pull Request
#440
Summary
Remove data source and location source JSONs from artifacts.
These JSONs are no longer provided from both CLI and WebAPI includes container image, but we can use them of course if we need because JSONs are still kept in the repo.
Changes
Essense of this PR is following:
data-sources/
fromItemGroup
, anddata-sources/json
would be created in publish directory.location-sources/
fromItemGroup
, andlocation-sources/json
would be created in publish directory.test-data-azure-emissions.json
is no longer set by default.dotnet tool run
for generating OpenAPI document.data-sources
andlocation-sources
.data-sources
andlocation-sources
, but they would be created into publish directory.This PR has other changes, but they are for test cases because they depends JSON data source and location source.
Note that I commented out test case for ElectricityMapsFree in both src/CarbonAware.WebApi/test/integrationTests/CarbonAwareControllerTests.cs and src/CarbonAware.WebApi/test/integrationTests/LocationsControllerTests.cs because IntegrationTestingBase.cs does not have configuration for ElectricityMapsFree. I will fix it after this PR.
Checklist
Are there API Changes?
No
Is this a breaking change?
This PR breaks current behavior. CASDK is no longer refer test-data-azure-emissions.json for data source and azure-regions.json for location source. CASDK requires the user to configure data source at least.