Copilot commented on code in PR #6462: URL: https://github.com/apache/texera/pull/6462#discussion_r3598612645
########## frontend/src/app/dashboard/service/user/flarum/flarum.service.spec.ts: ########## @@ -0,0 +1,95 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { TestBed } from "@angular/core/testing"; +import { FlarumService } from "./flarum.service"; +import { UserService } from "../../../../common/service/user/user.service"; + +describe("FlarumService", () => { + let service: FlarumService; + let httpMock: HttpTestingController; + + const mockUser = { uid: 42, email: "[email protected]", googleId: "secret-token" }; + const getCurrentUser = vi.fn(); + + beforeEach(() => { + getCurrentUser.mockReturnValue(mockUser); + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [FlarumService, { provide: UserService, useValue: { getCurrentUser } }], + }); + service = TestBed.inject(FlarumService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); + }); + + it("should be created", () => { + expect(service).toBeTruthy(); + }); + + describe("register", () => { + it("POSTs a user with a username derived from email local-part + uid", () => { + service.register().subscribe(); + + const req = httpMock.expectOne("forum/api/users"); + expect(req.request.method).toEqual("POST"); + expect(req.request.body.data.attributes).toEqual({ + username: "alice42", + email: "[email protected]", + password: "secret-token", + }); + expect(req.request.headers.get("Authorization")).toContain("Token "); + req.flush({}); + }); + + it("handles an email that contains multiple '@' by using only the first local-part segment", () => { + getCurrentUser.mockReturnValue({ uid: 7, email: "weird@[email protected]", googleId: "g" }); + + service.register().subscribe(); + + const req = httpMock.expectOne("forum/api/users"); + expect(req.request.body.data.attributes.username).toEqual("weird7"); + req.flush({}); + }); + }); Review Comment: The issue/PR metadata calls out exercising both success and error paths, but the register() tests only cover the happy path. Adding an error-path assertion helps ensure HttpClient errors are propagated (and prevents false positives where the Observable might be caught/handled later). ########## frontend/src/app/dashboard/service/user/flarum/flarum.service.spec.ts: ########## @@ -0,0 +1,95 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { TestBed } from "@angular/core/testing"; +import { FlarumService } from "./flarum.service"; +import { UserService } from "../../../../common/service/user/user.service"; + +describe("FlarumService", () => { + let service: FlarumService; + let httpMock: HttpTestingController; + + const mockUser = { uid: 42, email: "[email protected]", googleId: "secret-token" }; + const getCurrentUser = vi.fn(); + + beforeEach(() => { + getCurrentUser.mockReturnValue(mockUser); + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [FlarumService, { provide: UserService, useValue: { getCurrentUser } }], + }); + service = TestBed.inject(FlarumService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); + }); + + it("should be created", () => { + expect(service).toBeTruthy(); + }); + + describe("register", () => { + it("POSTs a user with a username derived from email local-part + uid", () => { + service.register().subscribe(); + + const req = httpMock.expectOne("forum/api/users"); + expect(req.request.method).toEqual("POST"); + expect(req.request.body.data.attributes).toEqual({ + username: "alice42", + email: "[email protected]", + password: "secret-token", + }); + expect(req.request.headers.get("Authorization")).toContain("Token "); + req.flush({}); + }); + + it("handles an email that contains multiple '@' by using only the first local-part segment", () => { + getCurrentUser.mockReturnValue({ uid: 7, email: "weird@[email protected]", googleId: "g" }); + + service.register().subscribe(); + + const req = httpMock.expectOne("forum/api/users"); + expect(req.request.body.data.attributes.username).toEqual("weird7"); + req.flush({}); + }); + }); + + describe("auth", () => { + it("POSTs credentials to the token endpoint", () => { + service.auth().subscribe(); + + const req = httpMock.expectOne("forum/api/token"); + expect(req.request.method).toEqual("POST"); + expect(req.request.body).toEqual({ + identification: "[email protected]", + password: "secret-token", + remember: "1", + }); + req.flush({}); + }); + }); Review Comment: The auth() suite currently only validates the POST body on success. Since the linked issue mentions covering success + error paths, consider adding an explicit HTTP error-path test to ensure errors propagate to subscribers. ########## frontend/src/app/dashboard/service/user/flarum/flarum.service.spec.ts: ########## @@ -0,0 +1,95 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { TestBed } from "@angular/core/testing"; +import { FlarumService } from "./flarum.service"; +import { UserService } from "../../../../common/service/user/user.service"; + +describe("FlarumService", () => { + let service: FlarumService; + let httpMock: HttpTestingController; + + const mockUser = { uid: 42, email: "[email protected]", googleId: "secret-token" }; + const getCurrentUser = vi.fn(); + + beforeEach(() => { + getCurrentUser.mockReturnValue(mockUser); + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [FlarumService, { provide: UserService, useValue: { getCurrentUser } }], + }); + service = TestBed.inject(FlarumService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); + }); + + it("should be created", () => { + expect(service).toBeTruthy(); + }); + + describe("register", () => { + it("POSTs a user with a username derived from email local-part + uid", () => { + service.register().subscribe(); + + const req = httpMock.expectOne("forum/api/users"); + expect(req.request.method).toEqual("POST"); + expect(req.request.body.data.attributes).toEqual({ + username: "alice42", + email: "[email protected]", + password: "secret-token", + }); + expect(req.request.headers.get("Authorization")).toContain("Token "); + req.flush({}); + }); + + it("handles an email that contains multiple '@' by using only the first local-part segment", () => { + getCurrentUser.mockReturnValue({ uid: 7, email: "weird@[email protected]", googleId: "g" }); + + service.register().subscribe(); + + const req = httpMock.expectOne("forum/api/users"); + expect(req.request.body.data.attributes.username).toEqual("weird7"); + req.flush({}); + }); + }); + + describe("auth", () => { + it("POSTs credentials to the token endpoint", () => { + service.auth().subscribe(); + + const req = httpMock.expectOne("forum/api/token"); + expect(req.request.method).toEqual("POST"); + expect(req.request.body).toEqual({ + identification: "[email protected]", + password: "secret-token", + remember: "1", + }); + req.flush({}); + }); + }); + + it("throws when no user is logged in", () => { + getCurrentUser.mockReturnValue(undefined); + expect(() => service.register()).toThrow(); + }); Review Comment: The no-user negative case only asserts register() throws. Since auth() dereferences the current user as well, it would be good to cover that both entry points fail fast when no user is logged in (matching the issue’s "no-user case" requirement). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
