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

Add support for SpaceAutomation #2545

Merged
merged 2 commits into from
May 4, 2021
Merged

Add support for SpaceAutomation #2545

merged 2 commits into from
May 4, 2021

Conversation

matkoch
Copy link
Contributor

@matkoch matkoch commented Jan 15, 2021

Let me know if anything is missing to support a new CI system.

@asbjornu
Copy link
Member

Thanks. Some tests would be nice.

@matkoch
Copy link
Contributor Author

matkoch commented Jan 15, 2021

Not exactly sure what you wanna test here. Do you want to add a sample CI?

@arturcic
Copy link
Member

@matkoch
Copy link
Contributor Author

matkoch commented Jan 15, 2021

There is no pull-request support yet. Honestly, other tests look more like repeating the implementation (or testing BuildAgentBase). Are you sure you don't want to have a sample CI instead?

@asbjornu
Copy link
Member

@matkoch, the tests ensure that the build server implementation output the expected stuff. We need tests for that. A sample CI would be fine as well, but does not alleviate the need for tests.

Base automatically changed from master to main January 31, 2021 12:46
@matkoch
Copy link
Contributor Author

matkoch commented Apr 30, 2021

Tests added. Sorry for the long delay!

@asbjornu
Copy link
Member

Awesome, @matkoch! If you could please rebase against HEAD of main, this should be good to go! :shipit:

@matkoch
Copy link
Contributor Author

matkoch commented May 3, 2021

Updated

Comment on lines 36 to 40
if (setEnvironmentTempFilePath != null && File.Exists(setEnvironmentTempFilePath))
{
File.Delete(setEnvironmentTempFilePath);
setEnvironmentTempFilePath = null;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this one is not needed here at all, as only the GH actions build agent is using a file, so it is needed only in the GH tests

buildServer = sp.GetService<SpaceAutomation>();
environment.SetEnvironmentVariable(SpaceAutomation.EnvironmentVariableName, "true");

setEnvironmentTempFilePath = Path.GetTempFileName();
Copy link
Member

Choose a reason for hiding this comment

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

I think this one is not needed here at all, as only the GH actions build agent is using a file, so it is needed only in the GH tests

{
private IEnvironment environment;
private SpaceAutomation buildServer;
private string setEnvironmentTempFilePath;
Copy link
Member

Choose a reason for hiding this comment

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

I think this one is not needed here at all, as only the GH actions build agent is using a file, so it is needed only in the GH tests

@matkoch
Copy link
Contributor Author

matkoch commented May 3, 2021

Updated.

@arturcic arturcic added this to the 5.6.10 milestone May 4, 2021
@arturcic arturcic merged commit 2e7ab29 into GitTools:main May 4, 2021
@mergify
Copy link
Contributor

mergify bot commented May 4, 2021

Thank you @matkoch for your contribution!

@github-actions
Copy link

🎉 This issue has been resolved in version 5.6.10 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

@matkoch
Copy link
Contributor Author

matkoch commented May 27, 2021

Just a patch increment?:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants