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

TerminalApp.Unit.Tests.manifest gets updated when building #12543

Closed
j4james opened this issue Feb 21, 2022 · 1 comment · Fixed by #12549
Closed

TerminalApp.Unit.Tests.manifest gets updated when building #12543

j4james opened this issue Feb 21, 2022 · 1 comment · Fixed by #12549
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@j4james
Copy link
Collaborator

j4james commented Feb 21, 2022

Windows Terminal version

Commit 09e9915

Windows build number

10.0.19041.1415

Other Software

No response

Steps to reproduce

  1. Checkout the latest source (commit 09e9915 or later).
  2. Build the solution.
  3. Look for any modified files in git status.

Expected Behavior

There shouldn't be any modified files.

Actual Behavior

The TerminalApp.Unit.Tests.manifest file is being updated with the same changes that were made to the WindowsTerminal.manifest file in PR #12462.

Shouldn't those changes have been committed at the same time as the WindowsTerminal manifest? Or if the unit test manifest is generated automatically, then maybe it shouldn't be committed at all.

Or am I the only one that's getting this?

@j4james j4james added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Feb 21, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Feb 21, 2022
@zadjii-msft
Copy link
Member

What, no, I always build the unit tests.... 😅

I think there's at least one open PR that's got a similar update in it snuck in, but if you'd like to submit a PR just for this tiny bit then that's fine.

Frankly those tests should probably be reworked into the broader localtests or fixed so they don't need to do that anymore, but 🤷

@ghost ghost added the In-PR This issue has a related PR label Feb 21, 2022
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Feb 22, 2022
@ghost ghost closed this as completed in #12549 Feb 22, 2022
@ghost ghost removed the In-PR This issue has a related PR label Feb 22, 2022
ghost pushed a commit that referenced this issue Feb 22, 2022
## Summary of the Pull Request

In PR #12462, a new `maxversiontested` entry was added to the
_WindowsTerminal.manifest_ file which now gets propagated to the
_TerminalApp.Unit.Tests.manifest_ file whenever a full build is
executed. This PR is just committing the updated test manifest.
 
## PR Checklist
* [x] Closes #12543
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #12543
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Feb 22, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants