forked from proEndreeper/RSWikiRedirector
-
Notifications
You must be signed in to change notification settings - Fork 2
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 unit tests and improve page identification logic #1
Open
thelearned1
wants to merge
27
commits into
SSyl:master
Choose a base branch
from
thelearned1:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
On the homepage of the Elder Scrolls wiki, there are several links which point towards main pages for each Elder Scrolls game. These pages have titles of the form "Portal:[Elder Scrolls Game]". In UESP, the corresponding page has the title '[Game]:[Game]', and there is no page with a title matching /^Portal:/ in all of the UESP wiki. This commit is intended to address this issue by adding another phase in the generation of the redirect link where, if the Wikia page name matches the pattern /^Portal:(.+)$/, the user is redirected to the page /\1:\1/, i.e. the UESP namespace is whatever the Wikia page is called in the Wikia "Portal" namespace.
This commit aims to address another major category of pages which the original extension leaves unaltered: typically, pages on the Skyrim wiki will belong to the "Skyrim:" namespace on the UESP wiki. Doing this cleanly requires a significant refactor of the code as previously written. As a result, the namespace and page name are now determined seperately, rather than all being found at once with a single regex. This has not caused any new bugs to emerge, at least not in my testing. Subsequent to this commit, two more changes are expected: * Tests will be created to verify that my revisions have not created any major bugs * One additional edge case will be accounted for --- the case when the user navigates directly to "elderscrolls.fandom.com" or "skyrim.fandom.com", which should redirect to the UESP home page and the Skyrim mainspace portal, respectively.
This commit creates unit tests for the procedure for coming up with a title for the redirected page. redirect page title. This required changes in three distinct stages: First, I refactored out this procedure into a separate function, now located in the file `getUespPage.js`. Second, because Firefox does not presently support modules in extensions, this is packaged (along with `background-module.js`) into the new background script at build time using esbuild (now registered as a dev dependency). Third, I added JestJS unit tests and corresponding test cases stored in JSON format in the newly-created `__tests__` directory. This also necessitated the creation of a second node script, `node test`. When this script is run, test results are output to `testResults.txt` (which is untracked).
Because there are now modules installed via Node Package Manager, it is probably advisable to have a .gitignore file that excludes node_modules and the output of the test script. This commit adds such a file.
This commit fixes an issue where, if the user navigates to 'skyrim.fandom.com' or 'elderscrolls.fandom.com' directly, instead of being correctly redirected to the UESP pages 'Skyrim:Skyrim' or 'Main Page', the wiki is instead searched for the string '/'.
The README will also need to be updated to reflect changes in how the extension is built, but I will wait until I have a response on this PR first. Sorry — totally spaced on that! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
This PR makes several improvements to testing, testability, and to the logic around which Wikia/Fandom pages correspond to which UESP pages.
Why
The overarching goal is to improve UX and to generally increase the utility of what is already an awesome extension.
That said, I do recognize that this PR is somewhat out of the blue. It seemed to me that there is already some logic in the present version of the extension that attempts to correctly identify the UESP namespace of a given ESWiki page based on its title. However, because the default strategy is to search UESP for whatever title is identified, the current iteration has some drawbacks for pages of certain types. My additions try to reduce some of those drawbacks.
As evidenced by the provided tests, this has been somewhat successful (though admittedly I wrote them :-P). Coverage at identifying the exact correct UESP name, or name that redirects to correct UESP page, increases from 27 to 96 of 274 pages (9.8% to 35%). See the "Testing" section for a thorough breakdown and description of my testing approach.
How
https://skyrim.fandom.com/wiki/Guild_Master's_Gloves
searches UESP for the page "Skyrim:Guild Master's Gloves")https://en.uesp.net/wiki/Main_Page
)Testing
I add tests only for the function that maps ES Wiki pages to UESP pages, since the rest of the extension remains unchanged.
There are three categories of tests from four data sources:
Tests were run with
Jest
, and are located in the__tests__
directory.Results
Results for the original extension were as follows:
Results for the variation in this PR:
I aggregated these data for the purpose of this PR; the raw outputs of the test suite can be found in the attachments under "testResults-old.txt" and "testResults-revised.txt", respectively.
Supplemental Note: Testing Methodology
I should start by noting that my baseline assumption was that the extension is correct only when it has successfully redirected an ESWiki page to the most closest-corresponding UESP page. Thus, I considered a case where the correct UESP page appeared in the search results a failure for the purpose of unit testing, despite this having only a slight negative impact on UX. That said, I constructed my tests such that a test case would pass if any of the following was true:
testResults-revised.txt
testSummary-old.txt