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

Update binlog path. #6

Merged
merged 1 commit into from
Aug 5, 2024
Merged

Update binlog path. #6

merged 1 commit into from
Aug 5, 2024

Conversation

veleek
Copy link
Member

@veleek veleek commented Aug 5, 2024

This fails to run because build exists an a bash script in the nunit-console root, so trying to create a file in build\NUnitConsole.binlog fails.

I missed this in my testing because I made the change as an afterthought to try and avoid needing to modify the .gitignore file. build/ is ignored by default so I figured that'd be a good place to put it without even thinking to try it out. ☹️

I used build-results to parallel test-results which is currently used for all test output, but we can put it wherever. I'll update the .gitignore with this folder.

This fails to run because `build` exists an a bash script in the `nunit-console` root, so trying to create a file in `build\NUnitConsole.binlog` fails.  

I missed this in my testing because I made the change as an afterthought to try and avoid needing to modify the `.gitignore` file.  `build/` is ignored by default so I figured that'd be a good place to put it without even thinking to try it out. ☹️ 

I used `build-results` to parallel `test-results` which is currently used for all test output, but we can put it wherever.  I'll update the `.gitignore` with this folder.
@veleek veleek requested a review from CharliePoole August 5, 2024 13:37
@veleek
Copy link
Member Author

veleek commented Aug 5, 2024

This is a reasonably trivial change, so I'm committing it to unblock testing on nunit-console but I'm happy to revert and modify.

@veleek veleek merged commit 8efc66a into main Aug 5, 2024
2 checks passed
@veleek veleek deleted the veleek/binlog-path branch August 5, 2024 13:40
@CharliePoole
Copy link
Member

Hmm... the AppVeyor Build is still not running at all when you merge into main. I'll look at the settings.

@CharliePoole
Copy link
Member

It was disabled. Fixed and triggered manually. FYI there were problems with AppVeyor building PRs. Next new PR will tell us if those are fixed.

@@ -196,7 +196,7 @@ public static class BuildSettings
BinaryLogger = new MSBuildBinaryLoggerSettings
{
Enabled = true,
FileName = "build/NUnitConsole.binlog",
FileName = "build-results/NUnitConsole.binlog",
Copy link
Member

Choose a reason for hiding this comment

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

Is there ever more than one file here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now, I don't believe so, but if we have something that locally builds on multiple platforms (maybe using docker or something) the I could imagine having a binlog for each platform.

@CharliePoole
Copy link
Member

CharliePoole commented Aug 5, 2024

OK... the AppVeyor build failed because the build is trying to produce a new production release for some reason. The package that is created is versioned as 3.18.1, which is actually our last production release. I would have expected it to be
3.18.2-devxxxxx instead. What happens if you run build -t Package locally? What gets created in the package directory?

BTW, everything that should run runs and passes, so that's good.

@veleek
Copy link
Member Author

veleek commented Aug 5, 2024

What happens if you run build -t Package locally? What gets created in the package directory?

A package called NUnit.Cake.Recipe.1.0.0-veleek-binlog1.nupkg gets created.

@CharliePoole
Copy link
Member

OK, I took a look on my Windows VM and I think we are having some versioning issues. I assume you have an upstream defined to use https://github.com/nunit/NUnit.Cake.Recipe/main.git. You should do a rebase of your feature branch on that upstream and fix any conflicts. At that point, once you commit any changes, you should start seeing packages created with version 1.0.1.

The same thing applies in your local copy of nunit.org, except the versions will be different.

Probably it would be simpler if you created feature branches under the nunit org rather than on a fork. We usually use names like issue-xxx where there is an issue or something descriptive for simple changes without an issue. But lets get this all merged first.

I'm afraid I did leave with the recipe in a bit of an unstable state. I didn't think you would need to change it. So you may find that the NUnit.ConsoleRunner.NetCore package doesn't build after you merge. If that happens, just comment out the entire package definition in build.cake for now. If I get a few hours I'll try to bring it up to date myself and let you know, but no guarantees for the moment as I have family stuff going on till the end of the week.

@CharliePoole CharliePoole added this to the 1.1.0 milestone Aug 7, 2024
@CharliePoole CharliePoole added the Bug Something isn't working label Aug 7, 2024
@veleek
Copy link
Member Author

veleek commented Aug 7, 2024

Should there be a dev00003 build published? I checked last night and didn't see one.

Alternatively, are you aware of a way I can build nunit-console using a locally built NUnit.Cake.Recipe package? I assume that I can add a local package source (or just drop the package into my local package chance manually), but I haven't had a chance to try and I don't know if that'll play well with dotnet cake calls.

@CharliePoole
Copy link
Member

I just published dev00007 with all your changes so far plus a fix to #7.

AppVeyor appears broken. See #8. Meanwhile, I can publish locally but I am not sure you can.

You can build and test locally if you have a copies of nunit-console and the cake recipe in sibling directories. See comment at the top of the build.cake file for nunit-console. However, this should not be needed if you use dev00007.

I'll be back in action on the weekend. :-)

See also email I sent you a little while ago.

@OsirisTerje
Copy link
Member

OsirisTerje commented Aug 7, 2024

@CharliePoole On the other repos we have moved over to only using Github actions. We have dropped both the Azure Devops Pipelines where they were they used, and the appveyor builds. In the NUnit repo there is a yml file for pushing to Myget.

It simplified things, and also, we experienced that Appveyor were lagging with updating their servers with the appropriate frameworks.

@CharliePoole
Copy link
Member

@OsirisTerje Right, that's one of the things I want to put into the overall plan for CI. I'll try to have a planning session with @veleek this weekend or after. In general, I'd like to get set up so that the windows build under GitHub actions is the one that does the publishing.

That said, I have never NOT needed a backup plan, so I wonder if we shouldn't keep AppVeyor running and available. (Currently, the backup is that I publish from my local machine.)

@CharliePoole
Copy link
Member

🎉 This issue has been resolved in version 1.1.0 🎉

The release is available on:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants