fix(thumbinal): `FIXED` surplus characters #559

OtavioVB · 2022-07-25T13:40:08Z

Commit para fixação da excedência de caracteres na thumbnail de respostas

Essa é uma pull request para fixação de um bug da thumbnail de respostas, veja a seguir o erro:

Como pode-se perceber os caracteres excederam o limite da área permitida, isso acontece pois o limite permitido é 50, no entanto, ele excede em 14 caracteres por causa do array de caracteres: Em resposta a , que não é levado em consideração pela linha de comando que será alterada nessa pull request.

Desse modo, foi reparado para margem de erro limite permitida.

Commit para fixação da excedência de caracteres na thumbnail de respostas...

vercel · 2022-07-25T13:40:14Z

@OtavioVB is attempting to deploy a commit to the TabNews Team on Vercel.

To accomplish this, @OtavioVB needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

filipedeschamps · 2022-07-25T18:30:29Z

@OtavioVB sensacional, parabéns pela primeira contribuição!

Uma sugestão: se você rodar os testes com npm test o que acontece? Os testes passam ou não? Infelizmente o CI não rodou aqui porque o deploy da Vercel travou o fluxo inteiro. Mas peço que execute o comando ali em cima para ver o que acontece.

1) Os testes passaram? Péssimo. Isso significa que não tem nenhum teste forçando esse erro e cobrindo o comportamento para evitar regressão.

2) Os testes não passaram? Ótimo. Falta agora consertar as novas expectativas.

E depois disso vamos seguir com esse PR.

Em paralelo, sugiro explorarmos outras formas de mensurar a lagura, pois pela quantidade de caracteres é algo falho, uma vez que cada letra possui uma largura, por exemplo:

AAAAAAAAAAAAAAAAAAAA (20 caracteres)
iiiiiiiiiiiiiiiiiiii (20 caracteres)

E daí vai depender se o título está usando muitas ou poucas letras de largura pequena.

Então o que poderíamos é usar a função measureText() para mensurar o tamanho final do texto e com isso decidir cortar ele com ... ou não.

tabnews.com.br/models/thumbnail.js

Line 56 in cbdaeb8

function measureText(string, fontSize = 32) {

Essa função foi usada para decidir a largura da caixa que fica por baixo do username:

tabnews.com.br/models/thumbnail.js

Line 83 in cbdaeb8

usernameWidth: measureText(content.username),

Onde o valor usernameWidth é usado no template.

Poderíamos ter essa mesma mensuração para o parent_title e até para as linhas do title e conseguir ao máximo aproveitar o espaço dentro da imagem.

OtavioVB · 2022-07-25T18:45:13Z

@filipedeschamps Pois é, eu não rodei o comando npm test, pois não tenho instalado e não entendo muito como funciona... Não mexo muito com JavaScript, mas irei tentar aqui baixar o npm e rodar o projeto (tomare que eu não faça nenhuma merda).

filipedeschamps · 2022-07-25T19:00:48Z

@OtavioVB leia o README do projeto 👍

OtavioVB · 2022-07-25T19:01:37Z

Eu li o README do projeto, quando dou o npm install aparece isso pra mim:

npm WARN config global `--global`, `--local` are deprecated. Use `--location=global` instead.
npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR!
npm ERR! While resolving: @primer/react@35.2.1
npm ERR! Found: react@18.1.0
npm ERR! node_modules/react
npm ERR!   react@"18.1.0" from the root project
npm ERR!   peer react@"*" from @bytemd/react@1.15.0
npm ERR!   node_modules/@bytemd/react
npm ERR!     @bytemd/react@"1.15.0" from the root project
npm ERR!   10 more (@primer/octicons-react, @primer/octicons-react, next, ...)
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^17.0.0" from @primer/react@35.2.1
npm ERR! node_modules/@primer/react
npm ERR!   @primer/react@"35.2.1" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: react@17.0.2
npm ERR! node_modules/react
npm ERR!   peer react@"^17.0.0" from @primer/react@35.2.1
npm ERR!   node_modules/@primer/react
npm ERR!     @primer/react@"35.2.1" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR! See C:\Users\Admin\AppData\Local\npm-cache\eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Admin\AppData\Local\npm-cache\_logs\2022-07-25T19_00_29_321Z-debug-0.log

OtavioVB · 2022-07-25T19:05:41Z

Dei um npm install --force acho que foi

ghost · 2022-07-25T19:22:53Z

Dei um npm install --force acho que foi

@OtavioVB

Qual versão do Node.js você está usando? Eu tive o mesmo problema (e um monte de gente também, na verdade).
No readme tá errado, a versão do node.js por enquanto pra funcionar tudo legal precisa ser 16.15.0.
Aí só rodar npm install mesmo. É melhor do que usar --force porque não tem risco de quebrar algo.

Já falaram disso nessa issue aqui (e em outras também).

OtavioVB · 2022-07-25T19:36:07Z

Eu tinha chegado a ver esta issue um momento, e acho que vou ter que baixar essa versão mesmo do npm, com npm install --force está retornando assim para mim:

raceback (most recent call last):
  File "docker-compose", line 3, in <module>
  File "compose\cli\main.py", line 81, in main
  File "compose\cli\main.py", line 200, in perform_command
  File "compose\cli\command.py", line 60, in project_from_options
  File "compose\cli\command.py", line 152, in get_project
  File "compose\cli\docker_client.py", line 41, in get_client
  File "compose\cli\docker_client.py", line 170, in docker_client
  File "docker\api\client.py", line 197, in __init__
  File "docker\api\client.py", line 221, in _retrieve_server_version
docker.errors.DockerException: Error while fetching server API version: (2, 'CreateFile', 'O sistema não pode encontrar o arquivo especificado.')
[1900] Failed to execute script docker-compose```

OtavioVB · 2022-07-25T20:52:47Z

Oi Filipe, você pediu para eu fazer o teste, consegui...

Apareceu isso como resultado do teste

C:\Users\Admin\Desktop\tabnews.com.br>npm test

> tabnews.com.br@1.0.0 test
> kill-port 3000 && npm run services:up && concurrently -s -k -n next,jest --hide next 'npm run next' 'jest --runInBand --verbose' && npm run services:stop

Process on port 3000 killed

> tabnews.com.br@1.0.0 services:up
> docker-compose -f infra/docker-compose.development.yml up -d

postgres-dev is up-to-date
mailcatcher is up-to-date
[jest] 'run' n�o � reconhecido como um comando interno
[jest] ou externo, um programa oper�vel ou um arquivo em lotes.
[2] 'next'' n�o � reconhecido como um comando interno
[2] ou externo, um programa oper�vel ou um arquivo em lotes.
[3] ''jest' n�o � reconhecido como um comando interno
[3] ou externo, um programa oper�vel ou um arquivo em lotes.
[jest] run exited with code 1
--> Sending SIGTERM to other processes..
--> Sending SIGTERM to other processes..
[3] 'jest exited with code 1
--> Sending SIGTERM to other processes..
[2] next' exited with code 1

filipedeschamps · 2022-07-26T17:10:45Z

@OtavioVB que erro interessante! Eu nunca vi isso e é ruim estar em português, pois isso dificulta pesquisar no Google.

Mas pelos logs eu vi que subiram os serviços do Docker e isso é ótimo, só falta entender o que de fato são as outras reclamações, pois parece que não está sendo encontrado nenhuma dependência: jest, next, ou o concurrently está com algum bug no seu sistema operacional.

Duas perguntas:

Você conseguiria colocar o seu sistema operacional em inglês para vermos a mensagem de erro nessa linguagem?
Você chegou a rodar npm install e ele chegou ao final sem nenhum erro?

OtavioVB · 2022-07-26T18:42:21Z

npm test

> tabnews.com.br@1.0.0 test
> kill-port 3000 && npm run services:up && concurrently -s -k -n next,jest --hide next 'npm run next' 'jest --runInBand --verbose' && npm run services:stop

Process on port 3000 killed

> tabnews.com.br@1.0.0 services:up
> docker-compose -f infra/docker-compose.development.yml up -d

postgres-dev is up-to-date
Starting mailcatcher ... done
[jest] 'run' is not recognized as an internal or external command,
[jest] operable program or batch file.
[2] 'next'' is not recognized as an internal or external command,
[2] operable program or batch file.
[3] ''jest' is not recognized as an internal or external command,
[3] operable program or batch file.
[jest] run exited with code 1
--> Sending SIGTERM to other processes..
--> Sending SIGTERM to other processes..
[3] 'jest exited with code 1
--> Sending SIGTERM to other processes..
[2] next' exited with code 1

irei rodar o npm install novamente, e lembro-me de ter dado tudo correto

npm install

> tabnews.com.br@1.0.0 prepare
> husky install

husky - Git hooks installed

up to date, audited 1233 packages in 5s

126 packages are looking for funding
  run `npm fund` for details

4 vulnerabilities (1 moderate, 1 high, 2 critical)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

C:\Users\Admin\Desktop\tabnews.com.br>

OtavioVB · 2022-07-26T19:01:33Z

Engraçado Filipe, eu rodei npm install -g e apareceu isso:

npm install -g

added 1 package, and audited 3 packages in 870ms

found 0 vulnerabilities


> tabnews.com.br@1.0.0 test
> kill-port 3000 && npm run services:up && concurrently -s -k -n next,jest --hide next 'npm run next' 'jest --runInBand --verbose' && npm run services:stop

Process on port 3000 killed

> tabnews.com.br@1.0.0 services:up
> docker-compose -f infra/docker-compose.development.yml up -d

postgres-dev is up-to-date
mailcatcher is up-to-date
[jest] 'run' is not recognized as an internal or external command,
[jest] operable program or batch file.
[2] 'next'' is not recognized as an internal or external command,
[2] operable program or batch file.
[3] ''jest' is not recognized as an internal or external command,
[3] operable program or batch file.
[2] next' exited with code 1
--> Sending SIGTERM to other processes..
[jest] run exited with code 1
--> Sending SIGTERM to other processes..
--> Sending SIGTERM to other processes..
[3] 'jest exited with code 1

filipedeschamps · 2022-07-26T20:05:58Z

Interessantíssimo! Meu maior palpite é em alguma incompatibilidade com o concurrently que está dentro do comando npm test. Você está usando Windows, correto?

ghost · 2022-07-26T20:08:19Z


> tabnews.com.br@1.0.0 test
> kill-port 3000 && npm run services:up && concurrently -s -k -n next,jest --hide next 'npm run next' 'jest --runInBand --verbose' && npm run services:stop

Process on port 3000 killed

> tabnews.com.br@1.0.0 services:up
> docker-compose -f infra/docker-compose.development.yml up -d

postgres-dev is up-to-date
mailcatcher is up-to-date
[jest] 'run' is not recognized as an internal or external command,
[jest] operable program or batch file.
[2] 'next'' is not recognized as an internal or external command,
[2] operable program or batch file.
[3] ''jest' is not recognized as an internal or external command,
[3] operable program or batch file.
[2] next' exited with code 1
--> Sending SIGTERM to other processes..
[jest] run exited with code 1
--> Sending SIGTERM to other processes..
--> Sending SIGTERM to other processes..
[3] 'jest exited with code 1

Eu recebo essa mesma mensagem de erro toda vez que rodo npm test

filipedeschamps · 2022-07-26T20:22:21Z

@nottja1mmm você está rodando no Windows?

OtavioVB · 2022-07-26T20:31:59Z

Sim estou usando Windows Filipe

aprendendofelipe · 2022-07-26T20:48:19Z

Eu também uso Windows. O que eu faço para executar os testes é rodar o npm run dev em um terminal e no outro chamar npx jest --runInBand

OtavioVB · 2022-07-26T21:02:29Z

Isso acontece quando não faço o npm install -g é normal?

filipedeschamps · 2022-07-26T21:04:52Z

Problemas relacionados:

open-cli-tools/concurrently#208

open-cli-tools/concurrently#167

you can use single quotes on Unix, but not on Windows

OtavioVB · 2022-07-26T21:06:07Z

@filipedeschamps Estou rodando os testes a partir do que o @aprendendofelipe falou

OtavioVB · 2022-07-26T21:06:36Z

Era pra estar acontecendo isso correto?

aprendendofelipe · 2022-07-26T21:08:36Z

Era pra estar acontecendo isso correto?

Não era não. Está dando isso mesmo se esperar o servidor subir antes de executar os testes?

OtavioVB · 2022-07-26T21:11:11Z

OtavioVB · 2022-07-26T21:11:27Z

Era pra estar acontecendo isso correto?

Não era não. Está dando isso mesmo se esperar o servidor subir antes de executar os testes?

Eu não tava com o servidor rodando hahashasdads

OtavioVB · 2022-07-26T21:12:49Z


      at Object.toEqual (tests/integration/api/v1/contents/[username]/[slug]/thumbnail/get.test.js:322:59)
          at runMicrotasks (<anonymous>)

 PASS  tests/integration/api/v1/recovery/post.test.js
 PASS  tests/integration/api/v1/contents/[username]/get.test.js
 PASS  tests/integration/api/v1/users/post.test.js
 PASS  tests/integration/api/v1/users/[username]/patch.test.js
 PASS  tests/integration/api/v1/contents/[username]/[slug]/get.test.js
 PASS  tests/integration/api/v1/contents/[username]/[slug]/tabcoins/post.test.js
 PASS  tests/integration/api/v1/sessions/post.test.js
 PASS  tests/integration/api/v1/recovery/patch.test.js
 PASS  tests/integration/api/v1/contents/get.test.js (8.225 s)
 PASS  tests/integration/api/v1/contents/[username]/[slug]/children/get.test.js
 PASS  tests/integration/api/v1/_use-cases/registration-flow.test.js
 PASS  tests/integration/api/v1/sessions/get.test.js
 PASS  tests/integration/api/v1/events/get.test.js
 PASS  tests/integration/api/v1/migrations/get.test.js
 PASS  tests/integration/api/v1/users/[username]/get.test.js
 PASS  tests/integration/api/v1/users/get.test.js
 PASS  tests/integration/api/v1/user/get.test.js
 PASS  tests/integration/api/v1/migrations/post.test.js
 PASS  tests/integration/api/v1/users/firewall.post.test.js
 PASS  tests/integration/api/v1/status/get.test.js

Summary of all failing tests
 FAIL  tests/integration/api/v1/contents/[username]/[slug]/thumbnail/get.test.js
  ● GET /api/v1/contents/[username]/[slug]/thumbnail › Anonymous user › "child" content with long "parent_title", long "body" and 0 "children"

    expect(received).toEqual(expected) // deep equality

    Expected: 0
    Received: 1

      320 |
      321 |       expect(response.status).toEqual(200);
    > 322 |       expect(Buffer.compare(benchmarkFile, responseBody)).toEqual(0); // has the same bytes
          |                                                           ^
      323 |     });
      324 |
      325 |     test('"child" of a "child" content with "parent_title"', async () => {

      at Object.toEqual (tests/integration/api/v1/contents/[username]/[slug]/thumbnail/get.test.js:322:59)
          at runMicrotasks (<anonymous>)


Test Suites: 1 failed, 24 passed, 25 total
Tests:       1 failed, 316 passed, 317 total
Snapshots:   0 total
Time:        69.19 s, estimated 325 s
Ran all test suites.

OtavioVB · 2022-07-26T21:15:09Z

Eu fiz isso por osmose, mas não entendi qual é a funcionalidade disso a fundo, eu sei que são testes automatizados, mas você poderia me dar um aprofundamento disso Filipe?

aprendendofelipe · 2022-07-26T21:23:25Z

Eu fiz isso por osmose, mas não entendi qual é a funcionalidade disso a fundo, eu sei que são testes automatizados, mas você poderia me dar um aprofundamento disso Filipe?

O @filipedeschamps dá uma boa introduzida aqui:

https://www.youtube.com/watch?v=LVxhiiE4nTE

filipedeschamps · 2022-07-26T21:29:07Z

Test Suites: 1 failed, 24 passed, 25 total
Tests: 1 failed, 316 passed, 317 total

Ótimo! Teste quebrado com sucesso 😍 nesse caso é o que eu comentei lá em cima sobre esperar que o teste quebrasse com essa sua nova implementação 👍

filipedeschamps · 2022-07-26T21:33:41Z

Em paralelo @aprendendofelipe você consegue verificar se o test abaixo roda no Windows? Coloquei aspas duplas:

"test": "kill-port 3000 && npm run services:up && concurrently -s -k -n next,jest --hide next \"npm run next\" \"jest --runInBand --verbose\" && npm run services:stop",

OtavioVB · 2022-07-26T21:37:28Z

show!! Muito bom estar aprendendo com vocês, mas na minha cabeça (leiga), não faz sentido eu quebrar o teste de forma que não passe no teste kkk

OtavioVB · 2022-07-26T21:38:05Z

Test Suites: 1 failed, 24 passed, 25 total
Tests: 1 failed, 316 passed, 317 total

Ótimo! Teste quebrado com sucesso 😍 nesse caso é o que eu comentei lá em cima sobre esperar que o teste quebrasse com essa sua nova implementação 👍

Agora qual é o próximo passo?

filipedeschamps · 2022-07-26T21:46:10Z

show!! Muito bom estar aprendendo com vocês, mas na minha cabeça (leiga), não faz sentido eu quebrar o teste de forma que não passe no teste kkk

haahha justo! Não vou conseguir me aprofundar por aqui, mas a questão é a seguinte:

Na branch main, os testes sobre a thumbnail estavam esperando que a imagem fosse da forma X.
Ou seja, se você rodar a mesma bateria de testes na main, todos eles irão passar 👍
Aí você fez uma implementação que muda o comportamento das thumbnails, e agora ao invés de resultar em X, as thumbnails estão resultando em Y.
Até aí tudo bem, o detalhe é que os testes ainda estão esperando que o resultado seja X, e ao não passarem, eles lhe avisaram que algo mudou e isso é ótimo para evitar regressões não esperadas.
Nesse caso era esperado a regressão, então temos que atualizar os testes para eles esperarem o resultado Y.

OtavioVB · 2022-07-26T21:56:41Z

Caraca @filipedeschamps que show!! Isso caiu no melhor momento para mim! Recentemente, iniciei meus estudos sobre testes, a importância, os testes que eu estavam fazendo era manuais com criação de classes em abstrações, enfim, mas não tinha entendido que tinha como funcionar desse jeito, que coisa incrível!!

Irei pesquisar como funciona e como realiza as implementações disso para .NET!! Achei sensacional

aprendendofelipe · 2022-07-27T00:49:05Z

Em paralelo @aprendendofelipe você consegue verificar se o test abaixo roda no Windows? Coloquei aspas duplas:

Roda sim @filipedeschamps, mas lembra que nos PRs #440 e #455 testamos o \" que funciona no Mac e no Windows, mas dava erro no CI?

filipedeschamps · 2022-07-28T00:37:48Z

Roda sim @filipedeschamps, mas lembra que nos PRs #440 e #455 testamos o \" que funciona no Mac e no Windows, mas dava erro no CI?

Oh noooo!!! 😂 precisamos achar uma solução para isso! Inaceitável o npm test não rodar no Windows.

filipedeschamps · 2022-10-11T23:20:27Z

Talvez a gente deveria trocar toda a implementação de thumbnail por isso: https://vercel.com/blog/introducing-vercel-og-image-generation-fast-dynamic-social-card-images

filipedeschamps · 2022-10-26T22:00:38Z

@OtavioVB estou fechando esse PR por conta do novo recurso da Vercel, mas agradeço muito a tentativa de contribuição e por ela refletiu em consertarmos o npm test no Windows 🤝

aprendendofelipe · 2023-06-01T02:06:02Z

@OtavioVB você foi citado no post comemorativo:

https://www.tabnews.com.br/FelipeBarso/tabnews-novo-ranqueamento-otimizacoes-e-mais 🎉

fix(thumbinal): FIXED surplus characters

c7e4c96

Commit para fixação da excedência de caracteres na thumbnail de respostas...

OtavioVB added the bug Comportamento diferente do esperado label Jul 25, 2022

filipedeschamps mentioned this pull request Jul 28, 2022

Fazer o comando npm test funcionar em todos os sistemas operacionais #592

Merged

aprendendofelipe mentioned this pull request Oct 13, 2022

Criar og:image automaticamente usando a nova biblioteca da vercel #782

Closed

filipedeschamps closed this Oct 26, 2022

filipedeschamps deleted the fix/thumbnail/title-1 branch October 26, 2022 22:00

aprendendofelipe mentioned this pull request May 25, 2023

Nova lib para geração das thumbnails #1425

Merged

fix(thumbinal): FIXED surplus characters #559

fix(thumbinal): FIXED surplus characters #559

Conversation

OtavioVB commented Jul 25, 2022

vercel bot commented Jul 25, 2022

filipedeschamps commented Jul 25, 2022

OtavioVB commented Jul 25, 2022

filipedeschamps commented Jul 25, 2022

OtavioVB commented Jul 25, 2022

OtavioVB commented Jul 25, 2022

ghost commented Jul 25, 2022

OtavioVB commented Jul 25, 2022

OtavioVB commented Jul 25, 2022

filipedeschamps commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

filipedeschamps commented Jul 26, 2022

ghost commented Jul 26, 2022 • edited by ghost Loading

filipedeschamps commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

aprendendofelipe commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

filipedeschamps commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

aprendendofelipe commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

aprendendofelipe commented Jul 26, 2022

filipedeschamps commented Jul 26, 2022

filipedeschamps commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

filipedeschamps commented Jul 26, 2022

OtavioVB commented Jul 26, 2022

aprendendofelipe commented Jul 27, 2022

filipedeschamps commented Jul 28, 2022

filipedeschamps commented Oct 11, 2022

filipedeschamps commented Oct 26, 2022

aprendendofelipe commented Jun 1, 2023

fix(thumbinal): `FIXED` surplus characters #559

fix(thumbinal): `FIXED` surplus characters #559

ghost commented Jul 26, 2022 •

edited by ghost

Loading