Repository navigation
Keep queued inline snapshots on the lines an accept moved them to, and keep four tests off the desktop - #929
Merged
Merged
Conversation
OptionsFormLauncherTests registered Ctrl+Alt+Shift+F13 to F17 with Windows for the length of each test, and asked what was held by trying to take it. A hot key is held for every process on the machine, so the tests failed wherever something else had one of those keys and took them from anything that wanted one while they ran. They are about which keys a save leaves bound, and all the desktop adds to that is refusing a combination that is already held. So the register is given a stand-in through KeyRegister's internal constructor, as KeyRegisterTests was: it keeps which id holds which combination and refuses a combination or an id that is taken, the two refusals RegisterHotKey gives. What is asserted is what the stand-in was left holding. Checked by running the class in Release: both tests pass, and nothing in the file calls the constructor that registers with Windows.
TrayVersionFileTest wrote and deleted %TEMP%\DiffEngineTray\version.txt, which is one file for the whole machine: a run of the suite removed the marker of the tray running there, and two of the tests running together read what the other had written. Write, Delete and TryRead each gain an internal overload that takes the file, and the public ones call them with FilePath, so the tray does what it did. Each test now uses a file in a folder named for that test instance, removed when it is done. Delete, which the tests only ever used to tidy up, has two tests of its own, and one more holds FilePath to where released libraries look for it without touching the file. Nothing else in the test projects reads or writes the marker: DiffEngine.Tests does not name TrayVersionFile, and the tray's session end test is handed its own action in place of Delete. Checked by running the class and then the whole of DiffEngineTray.Tests in Release: the real marker was absent before and after, and no folder of the tests' was left in the temp directory.
Five places in MenuBuilderTest called menu.Show(0, 0) to raise Opening, which is where the tracked items are built. A context menu is a top level window, so each one came up at the top left of the screen of whoever was at the machine, and one of the tests opened an item's drop down beside it. The menu is now made not top level before it is shown, and so is the drop down that is drawn. Neither then has a parent that is on a screen, and both still open, lay out, paint and close: Opening and Closed are raised as before, which is what four of the tests are about. Verify.WinForms draws a menu the same way, which is why the tests that only hand it a menu never showed one. A new test asks Windows, with IsWindowVisible, that a menu and a drop down opened this way are open and not visible. It was not run against the old call, since that would have put the menu on the desktop. Checked by running the class in Release: fifteen tests pass and no baseline moved, SnapshotThatWasNotWrittenDropDown's picture included.
RowText is the core's, and its tests were in DiffEngineViewer.Windows.Tests because the WinForms head is what asks for the start of a row. Nothing in them is about WinForms, and there they ran on Windows alone. Beside CellGridTests they run on all three platforms, as the grid's own tests do. The file is moved as it was, apart from the paragraph of its summary that explained why it was not there. No baselines go with it: these tests have none. Checked by a Release build of the solution and a Release run of DiffEngineViewer.Tests, which has the three tests in it and passes.
…sktop nobody is at Seven tests in WindowPlacementTests, ViewerFormRaiseTests and FrameWaitTests showed a ViewerForm the ordinary way, so it was the foreground window while they ran and took the keyboard of whoever was at the machine. Five of them also put a window nobody had made transparent on the screen, three of those maximised. Parking, which is what the hosts that post input got, does not reach these. A parked form is shown without activation, but what these tests do next is maximise, minimise, restore and raise, and WinForms asks each of Windows by a call that activates the window whatever it was shown with. A maximised window is on a display wherever it was put. And Raise ends in Activate, which is its point: a seam that left that out would be a test of something other than a raise. So the windows are made on a desktop of the test process's own (UnseenDesktop). Only the desktop that has the display is drawn or takes input, and on another one a window is shown, activated and sized by the same calls against the same displays. Nothing about the product changes and no assertion is weaker: each test asks what it asked before, of a window that really was raised or maximised. A thread with a window cannot be moved to another desktop, and the thread a test starts on has one already, COM's for its apartment, so each such test runs its window on a thread of its own and asserts on what comes back. Where a desktop cannot be made the tests run where they always did. The three placement tests that make a handle and never show the window are left as they were: there is nothing of them on the desktop. Checked by UnseenDesktopTests, which shows, maximises and activates a plain form there and finds it is never the foreground window, and by breaking Raise in a scratch build, which failed all four raise tests. The three classes ran five times each, and five more with DOTNET_PROCESSOR_COUNT=2.
… sites to A queued snapshot is named by its line, and accepting a snapshot moves every call site under it in the file. The entries left behind were then named by lines that had become other calls', and stayed that way until a run reported them again. Everything that finds an entry by its key had to guess: a re-run from a framework with no content in the entry was queued a second time beside it, a settle from a sibling call that had come to stand on the old line took an entry that was still failing, and two call sites only the line told apart were folded into one. In a batch the same thing happened inside the applier: every patch of a file was recorded against the file as it was read, so the second snapshot was looked for by a line the first had just moved, and an Append, which has nothing but its line, was refused wherever its member had two calls to choose from. A file with several such members took a run for each. The applier now says which lines an applied patch moved (InlineApplyResult.MovedFrom and MovedBy), read off the source before and after as SourceScan.Edited reads where to lex again. A batch asks each patch about the line the edits before it moved its call site to. And whoever owns the queue takes the file's remaining entries along once the outcomes are in (InlineQueue.Rebased): the tray after one accept and after each file of a bulk accept, the viewer the same, on its list where it stands, since an entry at another line is the same texts and the same diff under another key. A batch in the viewer names what it has still to do and a group's members by key, so those follow too, as does the entry being read. A tray driving a viewer's queue decides whether an accept failed by the entry still being under its key. After a snapshot that got shorter the one under it can come up onto that very line, so it now asks for an entry there that says why it failed. What this does not reach is an edit nothing reports: one made by hand, or an accept applied by another process, the IDE plugin among them. For those the questions about whose entry a line names are still asked, as before. Checked by AnAppliedPatchSaysWhichLinesItMoved, ALiteralThatShrinksMovesTheLinesUnderItUp, ABatchsResultsRebaseALineInOrder and AnAppendUnderAnEarlierEditIsAskedAboutTheLineItsCallIsOnNow against real files, four tests of InlineQueue.Rebased, RebasedEntriesTests in the viewer, WhatIsLeftOfAFileGoesToWhereItsCallSitesAre in the tray, and AnEntryThatMovesOntoTheAcceptedLineIsNotAFailedAccept over a real socket. TogetherIsWhatInTurnLeaves and ACarriedScanTellsEachPatchWhatLexingAgainWould now hold a batch to patches applied in turn at their moved lines, the second with the moves worked out a second way.
The entries found by line and the Append that needed a run a member are replaced by what rebasing leaves: edits nothing reports, a run under way during an accept, and a key changing under whoever shows the queue. The test hygiene items go, with what each fix left. claude.md gains the rebasing and the three ways a test now keeps off the desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two groups from
todo.md: keeping a queued inline snapshot's line right after an accept, and four tests that touched the desktop or the tray of whoever ran them.Needs attention before merging
UnseenDesktophas not run on CI. The tests that raise, maximise or minimise a window now run it on a desktop of the test process's own. WhereCreateDesktopfails they fall back to the ordinary desktop, as before, and one test is skipped. This PR'swindowsjob is the first run on a runner.InlineApplyResultgains public members:MovedFrom,MovedByandRebase(int).Hint rebasing
A queued snapshot is named by its line, and accepting a snapshot moves every call site under it in the file. The entries left behind were then named by lines that had become other calls', until a run reported them again.
SourceScan.LinesMoved).Append, which has nothing but its line, was refused wherever its member had two calls to choose from, so a file with several such members took a run each under accept-all.InlineQueue.Rebased): the tray after one accept and after each file of a bulk accept, the viewer the same. The viewer edits its list where it stands, with no entry diffed again, and brings along the selection and the keys a running batch holds.RemoteInlineHosttakes an accept to have failed only where the entry under its key says why. After a snapshot that got shorter, the one under it can come up onto the accepted line.Not reached, and now in
todo.md: an accept applied by another process (the IDE plugin) or a hand edit, which report no lines; and a run already under way during an accept, which reports the lines its call sites had (reasoned, not run).Test hygiene
OptionsFormLauncherTestsbinds its hot keys to a stand-in and registers nothing with Windows.TrayVersionFileTestuses a marker of its own, where it deleted the real%TEMP%\DiffEngineTray\version.txt.TrayVersionFilegains internal overloads taking the path.MenuBuilderTestopens its menus withTopLevelfalse, as Verify.WinForms does. No baseline moved. That nothing shows is asserted throughIsWindowVisible.ViewerFormRaiseTests, two ofWindowPlacementTestsand one ofFrameWaitTestsrun onUnseenDesktop, above. Parking could not do it: WinForms changes window state by calls that activate.RowText.Shown's tests move to the core's test project, so they run on all three platforms.Tests
dotnet build src --configuration Releaseis clean anddotnet test --solution src/DiffEngine.slnx --configuration Releasepasses on Windows: 3,326 tests, 0 failed, 34 skipped, and again with the process held to two cores. Each new rebasing test was seen to fail against a copy with the change disabled.Nothing under
native/changed, so no binaries PR will open.