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

test: testing-lib #37381

Merged
merged 46 commits into from
Sep 9, 2022
Merged

test: testing-lib #37381

merged 46 commits into from
Sep 9, 2022

Conversation

heiyu4585
Copy link
Contributor

@heiyu4585 heiyu4585 commented Sep 3, 2022

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English demoTest function
🇨🇳 Chinese 修改demoTest function 为 testing-lib

☑️ Self-Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2022

@afc163
Copy link
Member

afc163 commented Sep 4, 2022

冲突了

@MadCcc
Copy link
Member

MadCcc commented Sep 5, 2022

这个是为啥要改

@heiyu4585
Copy link
Contributor Author

冲突了

收到

@heiyu4585
Copy link
Contributor Author

heiyu4585 commented Sep 5, 2022

这个是为啥要改

demoTest function

@heiyu4585 heiyu4585 mentioned this pull request Sep 5, 2022
15 tasks
@heiyu4585 heiyu4585 force-pushed the dome-test branch 2 times, most recently from 40e56a2 to 8203e79 Compare September 5, 2022 12:11
@MadCcc
Copy link
Member

MadCcc commented Sep 6, 2022

snapshot 应该不变吧

@heiyu4585
Copy link
Contributor Author

heiyu4585 commented Sep 6, 2022

snapshot 应该不变吧

恩,我不确定, 主要是这个文件的改动 引起的,帮看下 /tests/shared/demoTest.tsx

</span>
</label>
<label
class="ant-radio-button-wrapper"
Copy link
Member

Choose a reason for hiding this comment

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

snapshot 能保持不变么,这样 diff 有点大。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

最终的的结果应该不大,主要是一些格式的东西,
image
(我会再把aria相关的属性 ignore掉,可能会再好一点)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我再找找看有没有格式配置

// @ts-ignore
child = child
.toString()
.replace(/ (aria-.*)=".+?"/g, ' $1=""')
Copy link
Member

Choose a reason for hiding this comment

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

这是干掉 aria- 么?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

恩,忽略掉

child = child
.toString()
.replace(/ (aria-.*)=".+?"/g, ' $1=""')
.replace(/ id=".+?"/g, ' $1=""');
Copy link
Member

Choose a reason for hiding this comment

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

这是做啥?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

@heiyu4585 heiyu4585 Sep 7, 2022

Choose a reason for hiding this comment

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

这个pr还继续么(要不我关了,去做新组件),巨佬 @afc163 我感觉我是不是走偏了

Copy link
Member

Choose a reason for hiding this comment

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

继续啊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

继续啊

ok

Copy link
Member

Choose a reason for hiding this comment

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

这个 tip 里是针对 id 不稳定的情况,id 稳定的情况还是希望能打出来了。现在去掉有点一刀切了。

Copy link
Contributor Author

@heiyu4585 heiyu4585 Sep 7, 2022

Choose a reason for hiding this comment

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

这个 tip 里是针对 id 不稳定的情况,id 稳定的情况还是希望能打出来了。现在去掉有点一刀切了。

dist 里已经把 ENV 固化了,所以它拿到的一定不会是 test env,而是按照 dist 固化出来的 dev or production,dist 里有些 test 就直接 ignore 了

@heiyu4585 heiyu4585 closed this Sep 7, 2022
@heiyu4585 heiyu4585 reopened this Sep 7, 2022
@zombieJ
Copy link
Member

zombieJ commented Sep 8, 2022

node env 报错是对的,因为 node 测试环境不应该有 jsdom。它需要对比的是 序列化 后的 html。这个需要走个分支处理一下。

@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #37381 (74aa516) into master (370c37c) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master    #37381   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          443       443           
  Lines         8176      8177    +1     
  Branches      2403      2403           
=========================================
+ Hits          8176      8177    +1     
Impacted Files Coverage Δ
components/_util/reactNode.ts 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@zombieJ zombieJ merged commit 01ca7c7 into ant-design:master Sep 9, 2022
@MadCcc MadCcc mentioned this pull request Sep 14, 2022
19 tasks
@heiyu4585 heiyu4585 mentioned this pull request Sep 23, 2022
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.

None yet

4 participants