Hi Tony, Thanks for the info. I'll be looking into test coverage as you suggested.
Here is some background regarding the progress for removing the web based solutions registry, aka "web.json5", and affected material. The main marker in this sequence is when your work with GPII-4382 was merged into universal master. Step 1: I removed the "web5.json" solutions registry, and ran all of the tests, including the production tests. This was done prior to GPII-4382 and the eye-opening result was that there were no test failures. Step 1a: I also grep'ed universal's "gpii" and "tests" folders for occurrences of the web solutions. They showed up only in the BrowserChannel tests, but, these tests use a mock win32 solutions registry that includes web solutions (e.g., cloud4chrome and cloud4firefox). Step 2: After the GPII-4382 code was merged, a number of tests failed with log messages about "rogue" settings in some preferences sets. The offending preferences sets included preferences for the now deleted web solutions, and since there was no longer a web solutions registry, the settings were deemed "rogue". Step 3: After discussion on the architecture list, these preferences sets, which referenced web-base solutions, were removed (e.g., vladimir.json5), and all tests passed again. Step 4: I'm currently looking for alternative solutions to use with the BrowserChannel tests. I've since discovered that these tests also use mock preferences sets that reference the "web" solutions from its corresponding mock win32 solutions registry (see step one). Also, the gpii-keys it uses are based on those mock solutions using keys named "chrome1", "chrome2", "chrome_and_firefox", and "firefox". One possibility here is that since these are tests of the BrowserChannel, the solutions, preferences, and keys don't matter as long as they are sufficient to show that the BrowserChannel is working. But, it might be misleading to use web solutions for that, even if they are mocks, and it would be better to align the tests with actual solutions, preferences sets, and gpii-keys. Step 5: While looking into the above, I discovered two old demo folders within "universal/testData/deviceReporter", namely "review3" and "secondPilots" that use web solutions, but look really old and obsolete. This material doesn't appear to be used by anything else in the system. On 2020-04-09 9:32 a.m., Tony Atkins wrote: > Hi, Joseph: > > Thanks for continuing to work to clean this up. I noticed similar > test errors when working on the replacement file for web.json5. I > dodged the question by simply getting my drop-in replacement working, > but it's a fair one. > > IMO we should only preserve the "web" tests if they are doing > something not already done elsewhere. For example, if we check a > pattern for all the different contexts, and we're just doing that > again for web.json5, we can lose that variation. If we're using the > web.json5 data to exercise code that isn't tested elsewhere, then the > tests probably need to be converted. > > If you can figure the "uniqueness" of the tests just from reading the > tests, awesome. Otherwise, the code coverage report might be a good > benchmark, i.e. compare a copy of the "reports" directory generated by > a test run against master (tons of these in CI) against the reports > generated when running the tests against your branch. If there are > gaps introduced when the tests are commented out, that's a sign they > might need to be converted or at least have their unique bits folded > into some other set of tests. > > Anyway, thanks again for looking at this. > > Cheers, > > > Tony > > On Mon, 6 Apr 2020 at 17:25, Joseph Scheuhammer <[email protected] > <mailto:[email protected]>> wrote: > > Hi, > > Further investigations of the use of the solutions listed in the 'web' > solutions registry, has revealed two test folders that are very > old and > likely could be removed. They are associated with the deviceReporter > and the directories in question are "review3" and "secondPilots". > They > are found as sub-folders in "gpii-universal/testData/deviceReporter/". > > The "review3" also indirectly references the "vladimir" > preferences set > via a group of mock solutions for vladimir for different contexts. > Recall that contexts are no longer supported. The names of the > solution > files are "installedSolutionsVladimirHotel.json5" and > "installedSolutionsValdimirSubway.json5" and so on. > > There is some documentation for the second pilots referenced from > a text > file in the "secondPilots" folder: > > * > > https://docs.google.com/document/d/1f4s6EUpLRR9ZMqQyJufmwx0l7HfzxdSNYusZa8vysSc/ > - This links to a document that brings up a dialog stating "File > is in > owner's trash". > > * http://wiki.gpii.net/w/Cloud4all_Second_Pilot_Phase > - Lots of secondary links > > * http://wiki.gpii.net/w/Cloud4all_Pilot_2_-_Set_up_and_installation > > There is one piece of documentation in "review3" that links to a long > google doc: > > https://docs.google.com/document/d/13fqxn2kpmLcdDkSnHKKpW3DtHaKVxP_zyA92-NqxWLU/edit > > Is any of this worth keeping? I've removed the 'web' solutions > and also > the preferences sets that depend on them, and am re-working some > tests. > But, it's not clear if above material directly depends on those > changes. There is probably some dependency, but if anyone has > knowledge > or opinions about this demo device reporter material, let me know. > > Thanks. > > -- > ;;;;joseph. > > 'The only reason for time is so that everything doesn't happen all > at once.' > - B. Banzai - > > _______________________________________________ > Architecture mailing list > [email protected] <mailto:[email protected]> > https://lists.gpii.net/mailman/listinfo/architecture > -- ;;;;joseph. 'The only reason for time is so that everything doesn't happen all at once.' - B. Banzai - _______________________________________________ Architecture mailing list [email protected] https://lists.gpii.net/mailman/listinfo/architecture
