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

Add Blazor WebAssembly tests in Boilerplate (#9059) #9060

Merged
merged 6 commits into from
Oct 30, 2024

Conversation

mjebrahimi
Copy link
Contributor

@mjebrahimi mjebrahimi commented Oct 30, 2024

Closes #9059

Summary by CodeRabbit

  • New Features

    • Introduced caching capabilities for Blazor WebAssembly applications.
    • Added new test classes for Identity and Localization functionalities.
  • Bug Fixes

    • Updated email confirmation logic to ensure proper redirection behavior.
    • Enhanced waiting mechanisms in phone confirmation tests.
  • Refactor

    • Improved test organization and clarity in Identity and Localization tests.
    • Adjusted caching and initialization logic in test setup.
  • Documentation

    • Updated comments and guidance in configuration files for clarity on server address settings.

Copy link

coderabbitai bot commented Oct 30, 2024

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The pull request introduces several modifications across various files, primarily enhancing the testing framework for Blazor WebAssembly applications. Key changes include the introduction of new test classes specifically for Blazor WebAssembly, modifications to existing test logic, and improvements to the caching mechanisms in the test setup. Additionally, adjustments to the GitHub Actions workflow for CI are made to refine test execution based on database options. The changes also include the commenting out of SignalR connection logic in the AppInitializer class.

Changes

File Change Summary
.github/workflows/bit.full.ci.yml Modified CI workflow to add filters for dotnet test commands for SQLite and SQL Server, replacing a single command with two separate commands.
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json Added exclusion path for advancedTests section to include PlaywrightCacheExtensions.cs.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppInitializer.cs Commented out ConnectSignalR method, disabling SignalR connection logic.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/appsettings.json Updated ServerAddress from "http://localhost:5030/" to "/".
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Extensions/PlaywrightCacheExtensions.cs Introduced a new file with extension methods for caching responses in Playwright, including methods for enabling asset caching and clearing cache.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/BlazorWebAssemblyTests.cs Added IdentityPagesTests and LocalizationTests classes for Blazor WebAssembly tests, overriding BlazorRenderMode property.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/IdentityPagesTests.cs Updated namespace to BlazorServer, reorganized using directives, and modified logic in SignUp, ForgotPassword, ChangeEmail, and ChangePhone methods.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/LocalizationTests.cs Updated namespace to BlazorServer and changed access modifier of SetCultureInBrowserContext from private to public.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Email/ConfirmationEmail.cs Changed WaitForRedirectOnMagicLink property value from false to true.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Identity/SettingsPage.Account.Email.cs Removed delay and page refresh logic from AssertConfirmEmailSuccess method.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Identity/SettingsPage.Account.Phone.cs Replaced Task.Delay(1000) with Page.WaitForTimeoutAsync(1000) in AssertConfirmPhoneSuccess method.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageTestBase.cs Made PageTestBase class abstract, added BlazorRenderMode property, and modified method visibility for test server configuration.
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/TestsInitializer.cs Enhanced Initialize method to include optional parameter for configuring test server settings, and added caching for Blazor WebAssembly mode.

Assessment against linked issues

Objective Addressed Explanation
Missing Blazor WebAssembly tests in Boilerplate (#9059)

🐰 "In the meadow, tests now bloom,
With Blazor's light, they chase the gloom.
Caching assets, swift and bright,
Our code hops forth, ready for flight!
With each new class, our joy expands,
In testing fields, we take our stands!" 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Outside diff range and nitpick comments (9)
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/BlazorWebAssemblyTests.cs (1)

1-15: Consider adding documentation for test setup requirements.

The test classes would benefit from XML documentation explaining:

  • Prerequisites for running WebAssembly tests
  • Any special configuration needed
  • Differences from server-side test behavior

Example addition:

/// <summary>
/// Tests for Blazor WebAssembly specific scenarios.
/// </summary>
/// <remarks>
/// Requires:
/// - WebAssembly runtime to be available
/// - Browser automation setup
/// </remarks>
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/appsettings.json (1)

Line range hint 13-13: Consider enhancing the ServerAddress configuration documentation.

While the current comment explains the relative URL usage, it could be helpful to add a note about the different implications for integrated vs standalone API modes.

Consider updating the comment to:

-    "ServerAddress_Comment": "If you're running Boilerplate.Server.Web project, then you can also use relative urls such as / for Blazor Server and WebAssembly",
+    "ServerAddress_Comment": "If you're running Boilerplate.Server.Web project with integrated API, use relative urls such as /. For standalone API, use the full server URL.",
🧰 Tools
🪛 Biome

[error] 2-2: Expected a property but instead found '//#if (api == "Integrated")'.

Expected a property here.

(parse)


[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/TestsInitializer.cs (1)

32-33: Consider improving comment formatting.

The inline comment could be more descriptive and follow C# XML documentation style for better consistency.

-            //Run assembly initialization test in BlazorWebAssembly mode to cache .wasm files
+            // Configure BlazorWebAssembly mode to enable .wasm file caching during assembly initialization
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Identity/SettingsPage.Account.Phone.cs (1)

74-74: Consider using more reliable wait conditions.

Good change to use Playwright's WaitForTimeoutAsync instead of Task.Delay. However, fixed timeouts can be fragile in UI tests.

Consider using more reliable wait conditions:

- await Page.WaitForTimeoutAsync(1000); //Wait for redirection to complete
+ // Wait for navigation or specific UI elements to indicate completion
+ await Page.WaitForURLAsync("**/settings"); // Or appropriate URL pattern
+ // Or wait for a specific element that indicates successful navigation
+ await Page.WaitForSelectorAsync("[data-testid='settings-page']", new() { State = WaitForSelectorState.Visible });

This would make the test more reliable and less dependent on timing conditions.

src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageTestBase.cs (2)

19-19: Add error handling for EnableBlazorWasmCaching.

Consider wrapping this call in a try-catch block to handle potential caching initialization failures gracefully.

-        await Context.EnableBlazorWasmCaching();
+        try
+        {
+            await Context.EnableBlazorWasmCaching();
+        }
+        catch (Exception ex)
+        {
+            throw new InvalidOperationException("Failed to initialize Blazor WebAssembly caching.", ex);
+        }

9-14: Consider adding XML documentation for the new Blazor WebAssembly features.

Adding XML documentation for the abstract class and BlazorRenderMode property would help developers understand:

  • The purpose of making the class abstract
  • When and how to override BlazorRenderMode
  • The implications of different render modes
+/// <summary>
+/// Base class for Blazor page tests supporting both Server and WebAssembly render modes.
+/// </summary>
 public abstract partial class PageTestBase : PageTest
 {
     public AppTestServer TestServer { get; set; } = new();
     public WebApplication WebApp => TestServer.WebApp;
     public Uri WebAppServerAddress => TestServer.WebAppServerAddress;
+    /// <summary>
+    /// Gets or sets the Blazor render mode for the test.
+    /// Override this property to switch between Server and WebAssembly modes.
+    /// </summary>
     public virtual BlazorWebAppMode BlazorRenderMode => BlazorWebAppMode.BlazorServer;
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/IdentityPagesTests.cs (2)

216-223: Consider adding edge case tests for email change flow.

The current implementation covers the main flows well. Consider adding tests for these scenarios:

  • Changing to an email that's already in use
  • Handling concurrent email change requests
  • Verifying old email notification of the change

Example test case:

[TestMethod]
[DataRow("DuplicateEmail")]
public async Task ChangeEmail_ToExistingEmail_ShouldFail()
{
    // Test implementation
}

Line range hint 1-324: Consider extracting common email verification logic.

The email verification logic is duplicated across different test methods. Consider creating a shared helper method or base class to handle common email verification tasks:

  • Email content assertions
  • Token retrieval
  • Magic link handling

Example refactor:

protected async Task<string> VerifyEmailAndGetToken(IEmailPage emailPage)
{
    var email = await emailPage.OpenEmail();
    await email.AssertContent();
    return await email.GetToken();
}
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Extensions/PlaywrightCacheExtensions.cs (1)

27-27: Use ! operator instead of is false for better readability

Using !cachedResponses.TryGetValue(...) is the conventional approach in C# and enhances code readability compared to cachedResponses.TryGetValue(...) is false.

Apply this diff:

-if (cachedResponses.TryGetValue(url, out var cachedResponse) is false)
+if (!cachedResponses.TryGetValue(url, out var cachedResponse))
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 388a298 and 0f03ddc.

📒 Files selected for processing (13)
  • .github/workflows/bit.full.ci.yml (2 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppInitializer.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/appsettings.json (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Extensions/PlaywrightCacheExtensions.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/BlazorWebAssemblyTests.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/IdentityPagesTests.cs (4 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/LocalizationTests.cs (2 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Email/ConfirmationEmail.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Identity/SettingsPage.Account.Email.cs (0 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Identity/SettingsPage.Account.Phone.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageTestBase.cs (3 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/TestsInitializer.cs (3 hunks)
💤 Files with no reviewable changes (1)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Identity/SettingsPage.Account.Email.cs
✅ Files skipped from review due to trivial changes (1)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppInitializer.cs
🧰 Additional context used
🪛 Biome
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/appsettings.json

[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-13: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

🔇 Additional comments (18)
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/BlazorWebAssemblyTests.cs (2)

1-4: LGTM! Proper namespace organization.

The namespace organization follows the correct hierarchical structure, and the required Client.Web dependency is properly imported.


5-9: Verify test inheritance and configuration.

The class inherits from the BlazorServer tests which promotes code reuse, and correctly overrides the BlazorRenderMode. However, we should verify that all test scenarios from the base class are applicable to WebAssembly context.

✅ Verification successful

Let me gather more specific information about the test classes.


Let me check one more aspect to understand the test initialization and setup.


Based on the gathered information, I can now provide the final response:

The test inheritance is properly implemented and safe for WebAssembly context

The test setup is well-designed for both Server and WebAssembly contexts:

  • The base class PageTestBase handles test initialization with proper mode configuration through BlazorRenderMode
  • The WebAssembly tests correctly override BlazorRenderMode to BlazorWebAssembly
  • Test initialization includes WebAssembly caching (EnableBlazorWasmCaching)
  • The test server setup is mode-agnostic and configures itself based on the render mode
  • No server-specific dependencies or scenarios are used in the identity tests
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the base class test methods to ensure compatibility with WebAssembly context

# Search for the base class implementation
echo "Checking base class implementation:"
ast-grep --pattern 'public class IdentityPagesTests {
  $$$
}'

# Look for any server-specific test scenarios
echo "Checking for server-specific test scenarios:"
rg -l "IServiceProvider|IServiceScope|HttpContext" --type cs

Length of output: 5242


Script:

#!/bin/bash
# Check the base class implementation and its test methods
echo "Checking BlazorServer.IdentityPagesTests implementation:"
rg -A 10 "class IdentityPagesTests" --type cs

# Check for any WebAssembly specific configurations or setups
echo "Checking for WebAssembly specific configurations:"
rg "BlazorWebAssembly|WebAssemblyHost" --type cs

# Check for any test setup methods
echo "Checking test setup methods:"
ast-grep --pattern 'public async Task TestInitialize() {
  $$$
}'

Length of output: 8782


Script:

#!/bin/bash
# Check the base class PageTest implementation
echo "Checking PageTest base class:"
ast-grep --pattern 'class PageTest {
  $$$
}'

# Check for any setup methods in the test classes
echo "Checking setup methods in test classes:"
rg -B 5 -A 10 "\[TestInitialize\]" --type cs

# Check for any WebAssembly specific test configurations
echo "Checking WebAssembly specific test configurations:"
rg -A 5 "AppTestServer|TestServer" --type cs

Length of output: 25410

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/appsettings.json (1)

3-3: LGTM! Good improvement to ServerAddress configuration.

Changing from absolute URL to relative path is a better practice as it:

  • Makes the configuration more environment-agnostic
  • Reduces cross-origin request complications
  • Aligns with the documented recommendation for Blazor Server and WebAssembly
🧰 Tools
🪛 Biome

[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-3: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 3-13: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageModels/Email/ConfirmationEmail.cs (1)

11-11: LGTM! Setting WaitForRedirectOnMagicLink to true improves test reliability.

The change ensures that tests wait for navigation to complete after clicking magic links, preventing potential race conditions in Blazor WebAssembly tests.

Let's verify the test execution time impact:

✅ Verification successful

Let me analyze the test timeout settings and related code to verify the impact of this change.


Setting WaitForRedirectOnMagicLink to true is correct and improves test reliability

The change is correct because:

  1. The base class TokenMagicLinkEmail uses this flag to wait for URL changes after clicking magic links
  2. For confirmation emails, redirection is expected as noted in the code comment: "destination page usually redirects to itself or another page"
  3. ResetPasswordEmail keeps it false since it doesn't redirect, while ConfirmationEmail needs redirection handling
  4. The implementation properly waits for the URL change using Page.WaitForURLAsync, preventing race conditions

The change aligns with the codebase's behavior and improves test stability by ensuring proper handling of navigation after clicking confirmation email links.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if there are any existing test timeouts or similar issues
# that might have been addressed by this change

# Look for timeout-related issues or race conditions in test files
rg -i "timeout|race condition|flaky|intermittent" --type cs

Length of output: 5924


Script:

#!/bin/bash
# Check the base class implementation and any related magic link handling
ast-grep --pattern 'class TokenMagicLinkEmail'

# Look for other similar email test classes
fd -e cs -x grep -l "WaitForRedirectOnMagicLink"

Length of output: 193568


Script:

#!/bin/bash
# Check the base class implementation and related magic link handling
rg -A 5 -B 5 "WaitForRedirectOnMagicLink" --type cs

Length of output: 6779

src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/LocalizationTests.cs (3)

5-5: LGTM! Good namespace organization.

The namespace change to Boilerplate.Tests.PageTests.BlazorServer improves test organization by clearly separating BlazorServer-specific tests. This aligns well with the PR objective of adding Blazor WebAssembly tests, as it provides a clear structure for platform-specific test organization.


Line range hint 11-65: LGTM! Well-structured localization tests.

The test implementation follows good practices:

  • Comprehensive coverage of localization scenarios
  • Clear test method names
  • Effective use of data-driven testing
  • Good separation of test data through CultureDataAttribute

20-20: Verify the necessity of making SetCultureInBrowserContext public.

While the method is used by the ConfigureBrowserContext attribute, consider if public access is truly needed. If this method is intended to be reused across test classes, consider moving it to a shared test utility class.

Let's check if this method is used outside this class:

✅ Verification successful

The public access modifier for SetCultureInBrowserContext is necessary

The method needs to be public because:

  1. It's invoked via reflection by ConfigureBrowserContextAttribute in PageTestBase.cs using BindingFlags.Public | BindingFlags.Instance
  2. The method is specifically referenced by name in the ConfigureBrowserContext attribute on the AcceptLanguageHeader test method
  3. The implementation is specific to localization testing and correctly placed in the LocalizationTests class
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any usage of SetCultureInBrowserContext outside this file
rg -l "SetCultureInBrowserContext" | grep -v "LocalizationTests.cs"

Length of output: 65


Script:

#!/bin/bash
# Let's first check the content of LocalizationTests.cs to understand the context better
rg -A 5 -B 5 "SetCultureInBrowserContext" "LocalizationTests.cs"

# Also search for any references to ConfigureBrowserContext attribute
rg -l "ConfigureBrowserContext"

# Let's also check if there are any other similar methods in test files
rg "BrowserNewContextOptions.*Locale.*=.*" 

Length of output: 590


Script:

#!/bin/bash
# Let's check the content of PageTestBase.cs to understand the relationship with ConfigureBrowserContext
rg -A 10 -B 10 "ConfigureBrowserContext" "src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageTestBase.cs"

# Let's also check the full content of LocalizationTests.cs to understand the class structure
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/LocalizationTests.cs"

Length of output: 4756

src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/TestsInitializer.cs (3)

9-10: LGTM: Imports are properly scoped.

The new imports are correctly placed within the advancedTests directive and support the Blazor WebAssembly testing functionality.


28-36: LGTM: Test configuration enhancement is well-implemented.

The addition of the configureTestConfigurations parameter and BlazorWebAssembly mode configuration provides the necessary setup for WebAssembly testing.


76-77: LGTM: Blazor WASM caching is properly implemented.

The EnableBlazorWasmCaching extension method is correctly placed before the sign-in process to ensure optimal caching behavior.

src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/PageTestBase.cs (4)

9-9: LGTM! Good architectural decision.

Making the class abstract enforces proper inheritance and prevents direct instantiation, which aligns well with its role as a base class for page tests.


37-40: LGTM! Well-structured configuration setup.

The configuration of BlazorMode through the build process is clean and maintainable.


78-78: LGTM! Consistent access level restrictions.

The restriction to public methods aligns with good encapsulation practices and maintains consistency with earlier changes.


14-14: Verify derived class implementations of BlazorRenderMode.

The virtual property with a default Server mode is well-designed, allowing flexibility while maintaining backward compatibility.

✅ Verification successful

BlazorRenderMode property is correctly implemented across test classes

The virtual property is being used as intended:

  • Base class PageTestBase provides default BlazorServer mode
  • BlazorWebAssemblyTests correctly overrides it to use BlazorWebAssembly mode
  • Other test classes (IdentityPagesTests, LocalizationTests) inherit the default server mode
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all classes that override BlazorRenderMode to ensure proper usage

# Test: Search for classes overriding BlazorRenderMode
ast-grep --pattern 'class $_ : PageTestBase {
  $$$
  public override BlazorWebAppMode BlazorRenderMode => $_
  $$$
}'

Length of output: 261


Script:

#!/bin/bash
# Let's try a different approach using ripgrep to find derived classes and BlazorRenderMode overrides

# First find all files that inherit from PageTestBase
rg "class.*:.*PageTestBase" -l

# Then search for BlazorRenderMode overrides with context
rg "override.*BlazorRenderMode" -A 2

Length of output: 872

.github/workflows/bit.full.ci.yml (1)

67-68: LGTM! Verify test coverage completeness.

The split test execution with specific filters aligns well with the PR objectives of adding Blazor WebAssembly tests. The separation provides better isolation and debugging capabilities.

Let's verify if all relevant tests are captured by these filters:

✅ Verification successful

Test filters are correctly configured and comprehensive

The test filters in the CI pipeline accurately cover all Blazor-related tests:

  • The first command captures BlazorServer tests through IdentityPagesTests and LocalizationTests in the BlazorServer namespace
  • The second command captures BlazorWebAssembly tests, which inherit from their BlazorServer counterparts
  • The inheritance structure shows proper test organization with PageTestBase as the foundation
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any Blazor-related test files that might be missed by the current filters
# Look for test files containing Blazor-related tests
rg -l "PageTests.*Blazor" --type cs

Length of output: 292


Script:

#!/bin/bash
# Let's examine the content of these files to understand the test structure
for file in $(rg -l 'PageTests.*Blazor' --type cs); do
    echo "=== $file ==="
    rg "namespace|class|public.*Test" "$file"
done

Length of output: 1624


Script:

#!/bin/bash
# Let's also check if there are any other test files in the PageTests directory
fd . -e cs --full-path ".*PageTests.*"

Length of output: 141


Script:

#!/bin/bash
# Let's check the actual test methods in BlazorWebAssemblyTests.cs
rg "public.*Test.*\(" "src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/BlazorWebAssemblyTests.cs"

Length of output: 111


Script:

#!/bin/bash
# Let's try a different approach to find all test files in the PageTests directory
find src -type f -path "*/PageTests/*" -name "*.cs"

Length of output: 1878


Script:

#!/bin/bash
# Let's check the inheritance hierarchy to understand test organization
rg "class.*:.*Test" --type cs

Length of output: 6138


Script:

#!/bin/bash
# Let's examine the base test class
rg "class PageTestBase" --type cs -A 5

Length of output: 835

src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/IdentityPagesTests.cs (2)

3-3: LGTM: Namespace organization aligns with PR objectives.

The namespace change to BlazorServer and additional using directives properly organize the test structure for separate Blazor WebAssembly testing.

Also applies to: 5-5, 7-7


96-105: Consider enhancing email content verification.

While the email confirmation flow is well-tested, consider adding specific assertions for the email content structure, such as:

  • Verifying the presence of required elements (header, body, footer)
  • Validating the token format
  • Checking the magic link URL structure
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json (1)

493-494: LGTM! Verify the PlaywrightCacheExtensions implementation.

The addition of PlaywrightCacheExtensions.cs to the advanced tests exclusion list aligns with the PR objective of adding Blazor WebAssembly tests. This ensures that the caching extensions are only included when advanced testing is enabled.

Let's verify the implementation of PlaywrightCacheExtensions:

✅ Verification successful

Implementation verified and properly integrated

The PlaywrightCacheExtensions implementation is well-designed and correctly integrated:

  • Implements caching for Blazor WebAssembly assets (.wasm, .pdb, .dat files)
  • Used in both TestsInitializer and PageTestBase for test setup
  • Provides efficient caching mechanism with regex pattern matching for WebAssembly resources
  • Includes proper cleanup through ClearCache()
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence and implementation of PlaywrightCacheExtensions

# Test 1: Verify the file exists and check its implementation
echo "Checking PlaywrightCacheExtensions implementation..."
rg -p "class.*PlaywrightCacheExtensions" -A 10

# Test 2: Check for Blazor WebAssembly related cache methods
echo "Checking for Blazor WebAssembly cache methods..."
rg -p "WebAssembly.*Cache" "src/Tests/Extensions/PlaywrightCacheExtensions.cs"

Length of output: 1246


Script:

#!/bin/bash
# Let's verify the complete implementation and usage context

# Check for the complete file content to understand the full implementation
echo "Checking complete PlaywrightCacheExtensions implementation..."
fd "PlaywrightCacheExtensions.cs" -t f --exec cat {}

# Check for any tests or usages of these extensions
echo "Checking for usages of PlaywrightCacheExtensions..."
rg "EnableBlazorWasmCaching|EnableAssetCaching" -A 2

Length of output: 5955

.github/workflows/bit.full.ci.yml Show resolved Hide resolved
Comment on lines +147 to +154
var resetPasswordEmail = await forgotPasswordPage.OpenResetPasswordEmail();
await resetPasswordEmail.AssertContent();
var token = await resetPasswordEmail.GetToken();
await resetPasswordPage.ContinueByToken(token);
break;
case "MagicLink":
resetPasswordEmail = await forgotPasswordPage.OpenResetPasswordEmail();
await resetPasswordEmail.AssertContent();
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance security assertions for password reset flow.

Consider adding security-focused assertions:

  • Verify token expiration handling
  • Assert that old tokens are invalidated after successful password reset
  • Check rate limiting behavior for reset password requests

Example addition:

// Add after successful password reset
await resetPasswordPage.AssertOldTokenInvalid(token);
await resetPasswordPage.AssertTokenExpiration();

@msynk msynk merged commit 239d52c into bitfoundation:develop Oct 30, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing Blazor WebAssembly tests in Boilerplate
2 participants